From 4a592f979571f44d7edb3019e8e3032a300b9443 Mon Sep 17 00:00:00 2001 From: William Mantly Date: Fri, 31 Jul 2026 01:22:08 -0400 Subject: [PATCH] Release 1.11.0: end-user catalog, access requests, nested groups Closes the end-user half of the directory and adds nested LDAP groups. The directory could describe the lab but could not tell anyone what they had or how to reach it, and several of the paths meant to do so were silently returning nothing: - GET /api/discovery/me resolved groups from req.user.groups, which does not exist (req.user carries memberOf), so it returned only isPublic resources for every human caller -- "My Services" was blank for everyone. The same read made isDirectoryAdmin() false for real admins. - The portal's "Discover More Services" called the admin-gated endpoint and swallowed the 403, so it never rendered for non-admins at all. - Services reported no address, because /me had reimplemented getMyAccess without its parent-walking resolution. Adds the catalog at /, self-service access requests, and admin access visibility (per-resource counts, and the reverse "what can this user reach"). Nested groups come in two halves. groupOfNames.member already accepts a group DN, so nesting needs no schema -- what it needs is resolution, which no released OpenLDAP performs. The all-in-one image therefore builds slapd from a pinned master commit for the nestgroup overlay, and the app computes the closure itself when pointed at a server without it. Both paths are covered. member-values is deliberately left out of nestgroup-flags: it expands `member` when reading a group, which destroys the distinction between "listed here" and "reachable through a nested group" and is not recoverable afterwards. Full suite green in both resolution modes: 215 passed, 2 skipped. Co-Authored-By: Claude Opus 5 --- API.md | 64 +++++ CHANGELOG.md | 41 +++ DEPLOYMENT.md | 11 + Dockerfile.openldap | 129 +++++++-- docker-entrypoint.sh | 78 +++++- docs/_config.yml | 5 + docs/concepts-accounts.md | 23 ++ docs/concepts-api-tokens.md | 4 +- docs/directory.md | 32 +++ docs/ldap.md | 71 ++++- nodejs/app.js | 3 + nodejs/conf/base.js | 15 + nodejs/models/access_request.js | 57 ++++ nodejs/models/group_ldap.js | 186 ++++++++++++- nodejs/models/index.js | 3 +- nodejs/models/resource.js | 67 +++-- nodejs/package-lock.json | 12 +- nodejs/package.json | 4 +- nodejs/routes/access_request.js | 266 ++++++++++++++++++ nodejs/routes/api_directory_admin.js | 229 ++++++++++++--- nodejs/routes/discovery.js | 39 ++- nodejs/routes/group.js | 90 ++++++ nodejs/routes/index.js | 7 + nodejs/routes/user.js | 17 +- nodejs/tests/access_request.test.js | 235 ++++++++++++++++ nodejs/tests/nested_group.test.js | 136 +++++++++ nodejs/utils/permission.js | 23 +- nodejs/utils/ui.js | 4 + nodejs/utils/user_groups.js | 56 ++++ nodejs/views/directory.ejs | 142 +++++++++- nodejs/views/groups.ejs | 104 +++++++ nodejs/views/landing.ejs | 399 ++++++++++++++++++++------- nodejs/views/profile.ejs | 8 +- 33 files changed, 2314 insertions(+), 246 deletions(-) create mode 100644 nodejs/models/access_request.js create mode 100644 nodejs/routes/access_request.js create mode 100644 nodejs/tests/access_request.test.js create mode 100644 nodejs/tests/nested_group.test.js create mode 100644 nodejs/utils/user_groups.js diff --git a/API.md b/API.md index e40dfca..73ab8ec 100644 --- a/API.md +++ b/API.md @@ -703,6 +703,10 @@ The authenticated user is automatically set as the group owner. { "results": true, "message": "Added user uid to group group." } ``` +Returns `409` if the user is already a member — common in practice, since +`groupOfNames` requires at least one member and so seeds whoever created the +group into it. + --- ### Remove User from Group @@ -716,6 +720,66 @@ The authenticated user is automatically set as the group owner. --- +### Nest a Group Inside Another + +**`PUT /api/group/:group/nested/:child`** — `app_sso_admin` or group owner + +Makes `:child` a member of `:group`, so everyone in `:child` is a member of +`:group` at any depth. + +**Response:** +```json +{ "results": { "cn": "group", "member": ["..."] }, "message": "Nested child inside group." } +``` + +**Errors:** + +| Status | When | +|--------|------| +| `400` | `:group` and `:child` are the same group | +| `409` | already nested, or the nesting would create a loop (`:child` already contains `:group`, directly or transitively) | + +--- + +### Un-nest a Group + +**`DELETE /api/group/:group/nested/:child`** — `app_sso_admin` or group owner + +**Response:** +```json +{ "results": { "cn": "group", "member": ["..."] }, "message": "Removed child from group." } +``` + +**Errors:** + +| Status | When | +|--------|------| +| `409` | `:child` is the only member — `groupOfNames` requires at least one | + +--- + +### Effective Membership + +**`GET /api/group/:group/effective`** — Any authenticated user + +Who a group actually grants. `direct` is users listed on the group itself +(never groups); `nestedGroups` is what is nested into it; `effective` is every +user reachable through the whole chain. + +**Response:** +```json +{ + "results": { + "cn": "app_gitea_access", + "direct": ["cn=alice,ou=people,dc=example,dc=com"], + "nestedGroups": [{ "cn": "developers", "dn": "cn=developers,ou=groups,dc=example,dc=com" }], + "effective": ["cn=alice,ou=people,dc=example,dc=com", "cn=bob,ou=people,dc=example,dc=com"] + } +} +``` + +--- + ### Delete Group **`DELETE /api/group/:group`** — `app_sso_admin` or group owner diff --git a/CHANGELOG.md b/CHANGELOG.md index 091e090..47d10b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,47 @@ All notable changes to this project are documented here. Format loosely follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versions correspond to git tags (`vX.Y.Z`) and `nodejs/package.json`'s `version`. +## [1.11.0] - 2026-07-31 + +Closes the end-user half of the directory. The admin side could describe the lab; the user side could not tell anyone what they had or how to use it, and several of the paths meant to do so were silently returning nothing. + +### Fixed +- **`GET /api/discovery/me` returned only `isPublic` resources for every human caller.** It resolved the caller's groups from `req.user.groups`, which does not exist — `req.user` is a `User` carrying `memberOf` (DNs). The empty list failed open into "no group-granted resources", so "My Services" on the profile page and the portal's service list were blank for everyone. The same bug made `isDirectoryAdmin()` false for real directory admins, silently downgrading them to the public metadata projection. Group CNs now come from `utils/user_groups.js`. +- **The portal's "Discover More Services" was dead for every non-admin.** It called the admin-gated `directory-admin/resources` and swallowed the 403 into an empty array — so the one discovery feature never rendered for the audience it existed for. It now calls `/api/discovery/resources`. +- **Services reported no address.** `/api/discovery/me` had reimplemented `Resource.getMyAccess` without its parent-walking address resolution, leaving clients to guess `address || ip`, which is exactly wrong for a service that is reached at its host's IP. Both paths now share `Resource.withResolvedAddress()`. +- **Approving access for a user already in the target group threw a 500** and left the request stuck pending. `groupOfNames` requires at least one member, so a resource's auto-created groups are seeded with the creator's DN; the grant is now idempotent. +- **`DELETE /api/directory-admin/resources/:id` deleted the resource before its edges and group links.** With no transaction, a failure mid-way orphaned rows pointing at a nonexistent id — invisible in the UI and poisonous to `getGraph()`. Dependents go first now. +- `PUT /api/directory-admin/resources/:id` validated the body only after loading the row, and carried a dead if/else whose branches were identical. +- `/api/directory-admin/audit-logs` shelled out to `tail` three times via `execSync`; replaced with a bounded async file read (no `child_process`, at most the trailing 256 KB). + +### Added +- **End-user catalog at `/`**, and the first ungated nav item — previously every nav entry was admin-only and a normal user had no signposted destination. Search/filter, per-kind icons, and a **how to reach it** block per card: the URL for a service, the SSH invocation for a host (using the jump-host `uid_-_slug@host` grammar when `directory.jumpHost` is configured). +- **Self-service access requests** — `AccessRequest` model plus `/api/access-requests` (create, list own, list decidable, approve, deny, withdraw). Approving performs the LDAP group add, so LDAP remains the access-control truth. Requests target a resource's `member`-level group, never its `_admin` one. Replaces the "coming soon" stub. +- **Admin access visibility**: an Access column on the directory table showing member and group counts (and flagging links whose LDAP group has been deleted), plus a "what can this user reach" lookup — the reverse question, which previously had no UI at all. Backed by `GET /api/directory-admin/access-summary` and `/user-access/:uid`. +- `conf.directory` — `jumpHost` and `defaultSshPort`, the connection conventions the catalog renders. +- `tests/access_request.test.js` — the request → approve → grant-is-real loop end to end, including the regression guard for the `user.groups` bug. + +### Added — nested groups +- **A group can now contain another group.** `groupOfNames.member` accepts any DN, so nesting needs no new schema; what it needs is *resolution*, which no released OpenLDAP performs — `memberOf` and `(member=X)` both return direct membership only. Two halves: + - **Server-side**: the all-in-one image now builds slapd from a pinned OpenLDAP master commit (`350e9eb3`) to get the **`nestgroup`** overlay (ITS#10161), enabled with `member-filter memberof-filter memberof-values`. `member-values` is deliberately omitted — it expands `member` when reading a group, which destroys the distinction between "listed here" and "reachable via nesting" and is not recoverable afterwards. `pw-sha2` is built from contrib in the same stage; without it every existing `{SSHA512}` password would be unverifiable. + - **Client-side**: `Group.list(dn)` computes the transitive closure itself (cycle-detected, depth-capped) when the server can't, selected by `conf.ldap.nestedGroupsServerSide` — which `docker-entrypoint.sh` derives from probing for `nestgroup.so` rather than hardcoding. Both paths are covered by the full suite. +- `PUT`/`DELETE /api/group/:group/nested/:child` and `GET /api/group/:group/effective`, plus a **Nested** tab on each group card. Cycles are refused (409) rather than silently depth-truncated. +- **`app_super_admin` is now seeded** (it never was) and nested into `app_sso_admin` / `app_sso_invite` / `app_sso_oauth_admin`, so the privilege is real LDAP membership visible to SSSD and sudo — not just a special case in `utils/permission.js`. Not nested into `app_sso_service_account`, which marks non-person accounts rather than granting anything. +- Creating a directory resource nests `app_super_admin → _admin` and `_admin → _access`. Both previously required adding every super admin to every new group by hand, so they drifted. +- `ldap_group_nesting_level = 5` in ldap-client's SSSD template, for hosts pointed at a server without `nestgroup`. Against the bundled slapd the existing `memberof=` access filter is already transitive, so SSH login inherits nesting for free. + +### Fixed +- `PUT /api/group/:group/:uid` returned a bare **500** when the user was already a member — common, since `groupOfNames` requires a member and so seeds whoever created the group. Now a 409 that says so. +- Un-nesting (or removing) the last member of a group returned a 500 `ObjectClassViolationError`; now a 409 explaining that a group must keep at least one member. +- `GET /api/user/me` derived `isAdmin` from `memberOf`, which is only transitive when `nestgroup` is present. Against a stock server an admin holding their group via nesting would get `isAdmin=false` and lose the entire admin UI while still passing every server-side permission check. +- `utils/permission.js`'s `byGroup` checked `group.member.includes(user.dn)` per group, seeing only direct membership. +- `/api/directory-admin/access-summary` counted `member` values; it now counts the transitive closure, which matters precisely because `app_super_admin` is nested into every resource's admin group. +- Broken `api.html` link in the published docs (`API.md` lives at the repo root, so Jekyll never rendered one); pointed at the source, and added an API entry to the docs nav. + +### Changed +- `@simpleworkjs/directory-schema` bumped to `^1.1.0`, which declares the ten metadata keys the admin form has always written but the schema never listed (`port`, `externalPort`, `isExternalReachable`, `os`, `gitRepo`, `isCurrentSite` as public; `vmid`, `macAddress`, `installPath`, `systemdService` as admin-only). Undeclared keys are dropped for non-admin callers, which blanked the portal's `OS:` field, hid every service's port from users, and left machine tokens unable to read the port mapping the firewall consumer exists to render. +- Resource metadata now includes `icon` and `tagline`, collected on the admin form (with a live icon preview) and rendered on the catalog cards. + ## [1.10.0] - 2026-07-30 ### Added diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index 617246d..31f00f1 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -336,6 +336,17 @@ OAuth client). Note: re-running bootstrap resets the bootstrap-admin and service-account passwords to the values in `./config/sso-secrets.js`; non-theta OAuth clients live in SSO Redis and are preserved by the volume. +> **Note — the bundled slapd is built from source.** The all-in-one image +> compiles OpenLDAP from a pinned upstream commit to get the `nestgroup` +> overlay (nested groups; see `docs/directory.md`), because no 2.6.x release +> ships it. One consequence: master uses **LMDB 1.0.0**, whose on-disk format is +> mutually unreadable with the 0.9.x in OpenLDAP 2.6.x +> (`MDB_INVALID: File is not an LMDB file`). Moving a directory between a 2.6.x +> image and this one is a `slapcat` → `slapadd` reload, not a restart — the same +> shape as "Restore — LDAP only" above. Verify after a rebuild: +> `docker compose logs sso-manager | grep nestgroup` should report the overlay +> as available. + --- ## Method 2: Bare metal (Debian/Ubuntu) diff --git a/Dockerfile.openldap b/Dockerfile.openldap index b795690..e2544cc 100644 --- a/Dockerfile.openldap +++ b/Dockerfile.openldap @@ -33,39 +33,118 @@ RUN if [ -n "$GIT_COMMIT" ]; then \ && git rev-parse --short HEAD > /commit.txt; } 2>/dev/null || echo unknown > /commit.txt; \ fi +# ── OpenLDAP from source ───────────────────────────────────────────────────── +# We build slapd from OpenLDAP master rather than installing Alpine's packages, +# for exactly one feature: the `nestgroup` overlay (ITS#10161, Howard Chu, +# 2024-03-21), which evaluates nested groups server-side. Nothing in any 2.6.x +# release can do this -- verified: 2.6.13 ships 26 overlay modules and +# nestgroup is not among them -- and the alternative is resolving nesting +# separately in every consumer (this app, SSSD on each host, jump-host, proxy), +# where any consumer that forgets silently under-grants access. +# +# Consequence to know about: master ships LMDB 1.0.0, whose on-disk format the +# 0.9.x used by 2.6.x cannot read, and vice versa +# ("MDB_INVALID: File is not an LMDB file"). Moving an existing directory onto +# this image is a slapcat/slapadd migration, not a restart. See DEPLOYMENT.md. +FROM node:20-alpine AS ldapbuild + +# groff is not optional despite producing nothing we ship: the build descends +# into doc/man unconditionally and its Makefile calls soelim, which groff +# provides. Without it the whole `make` fails at the man-page stage +# ("soelim: not found") long after slapd itself has compiled fine. +RUN apk add --no-cache \ + build-base autoconf automake libtool \ + openssl-dev cyrus-sasl-dev \ + git make pkgconf util-linux-dev groff + +# Pinned to an exact commit, not a branch tip. This is the directory server the +# whole lab authenticates against; an unpinned `master` would mean every image +# rebuild silently ships whatever landed upstream that morning, and a bad day on +# master would take out logins with no way to tell what changed. +# +# TODO: drop this whole from-source stage once nestgroup ships in a release. +# It is master-only today (ITS#10161, 2024-03-21); the 2.7 roadmap has slipped +# from Fall 2024 to Fall 2025 and is still unreleased. When 2.7 lands with +# nestgroup, revert to `apk add openldap openldap-overlay-nestgroup ...` -- +# the entrypoint already probes for nestgroup.so and needs no change, and the +# app already keys off app_ldap__nestedGroupsServerSide either way. +ARG OPENLDAP_COMMIT=350e9eb38b2270c2bad97c61ee02e85fb8f3196d + +WORKDIR /src +RUN git init -q . \ + && git remote add origin https://git.openldap.org/openldap/openldap.git \ + && git fetch -q --depth 1 origin "${OPENLDAP_COMMIT}" \ + && git checkout -q FETCH_HEAD \ + && git rev-parse HEAD > /opt-openldap-commit.txt + +# Overlays are built as loadable modules (=mod) because docker-entrypoint.sh +# `moduleload`s them individually; nestgroup joins that set. +RUN ./configure \ + --prefix=/opt/openldap \ + --enable-slapd \ + --enable-modules \ + --enable-mdb \ + --enable-memberof=mod \ + --enable-refint=mod \ + --enable-ppolicy=mod \ + --enable-dynlist=mod \ + --enable-nestgroup=mod \ + --enable-syncprov=mod \ + --enable-auditlog=mod \ + --with-tls=openssl \ + --with-cyrus-sasl \ + && make depend \ + && make -j"$(nproc)" \ + && make install + +# pw-sha2 provides {SSHA512}, which every existing user password is stored as. +# It lives in contrib and is not covered by the configure flags above, so it is +# built separately against the just-built tree -- omitting it would make every +# user password unverifiable. +RUN cd contrib/slapd-modules/passwd/sha2 \ + && make prefix=/opt/openldap OPENLDAP_SRC=/src \ + && cp .libs/pw-sha2.so* /opt/openldap/libexec/openldap/ + FROM node:20-alpine -# Install OpenLDAP and required packages. -# Alpine splits OpenLDAP into many small subpackages; there is no catch-all -# "openldap-overlays" package. We install exactly the backends/overlays/modules -# the app depends on: -# openldap-back-mdb : the mdb backend (slapd.conf uses `database mdb`) -# openldap-overlay-ppolicy : ppolicy module + overlay (account locking) -# openldap-overlay-memberof : reverse group membership -# openldap-overlay-refint : referential integrity on group members -# openldap-passwd-sha2 : pw-sha2 module ({SSHA512} user password hashing) -# Note: Alpine does NOT ship a ppolicy.schema file — on OpenLDAP 2.6 the ppolicy -# schema is built into ppolicy.so and registered when the module loads, so -# docker-entrypoint.sh loads it via `moduleload ppolicy` (no schema include). -# openssl : used by docker-entrypoint.sh to generate a JWT secret +# Runtime libraries the from-source slapd links against, plus the app's own +# deps. No openldap* packages here: everything LDAP comes from /opt/openldap. +# libltdl (module loading -- slapd is useless without it, since every overlay +# is a loadable module) and libuuid are pulled in by the source build but are +# NOT dependencies of anything else here, so they must be named explicitly; +# omitting them fails at runtime with "Error relocating ... lt_dlopenext: +# symbol not found", not at build time. RUN apk add --no-cache \ - openldap \ - openldap-clients \ - openldap-back-mdb \ - openldap-overlay-ppolicy \ - openldap-overlay-memberof \ - openldap-overlay-refint \ - openldap-overlay-syncprov \ - openldap-overlay-auditlog \ - openldap-passwd-sha2 \ + openssl \ + libsasl \ + libltdl \ + libuuid \ dumb-init \ bash \ - openssl \ redis \ && rm -rf /var/cache/apk/* -# The openldap package already creates the `ldap` user/group, which slapd runs -# as (see -u ldap -g ldap in docker-entrypoint.sh). Nothing to add here. +COPY --from=ldapbuild /opt/openldap /opt/openldap +# Which upstream commit this slapd was built from — so a running container can +# answer "what am I actually running" without rebuilding. +COPY --from=ldapbuild /opt-openldap-commit.txt /opt/openldap/COMMIT + +# The Alpine openldap package used to create these; nothing does now, and +# docker-entrypoint.sh runs slapd as -u ldap -g ldap. +RUN addgroup -S ldap 2>/dev/null || true \ + && adduser -S -D -H -G ldap ldap 2>/dev/null || true + +# docker-entrypoint.sh invokes slapd/slappasswd/ldapadd/ldapsearch by bare name +# and probes a list of candidate module directories, so putting the from-source +# tree first on PATH is all that is needed to redirect it. Schemas are symlinked +# into the conventional location because the entrypoint's slapd.conf includes +# /etc/openldap/schema/*.schema, and the app's own schemas (theta42, sudo, +# openssh-lpk) are copied there too. +ENV PATH="/opt/openldap/bin:/opt/openldap/sbin:/opt/openldap/libexec:${PATH}" +RUN mkdir -p /etc/openldap/schema \ + && for f in /opt/openldap/etc/openldap/schema/*.schema; do \ + ln -sf "$f" "/etc/openldap/schema/$(basename "$f")"; \ + done WORKDIR /app diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 0980608..c17a98f 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -76,11 +76,22 @@ fi # ── Locate the OpenLDAP module directory ──────────────────────────────────── # slapd.conf needs `modulepath` to find pw-sha2/ppolicy/memberof/refint. The # path varies by distro; auto-detect rather than hardcode. +# /opt/openldap/libexec/openldap is first: that is the from-source build (see +# Dockerfile.openldap), which is the only one carrying the nestgroup overlay. MODULE_PATH="" -for p in /usr/lib/openldap /usr/lib/ldap /usr/local/lib/openldap /opt/local/lib/openldap; do +for p in /opt/openldap/libexec/openldap /usr/lib/openldap /usr/lib/ldap /usr/local/lib/openldap /opt/local/lib/openldap; do if [[ -d "$p" ]]; then MODULE_PATH="$p"; break; fi done +# Nested-group support is only available when slapd was built with the +# nestgroup overlay. Detect rather than assume, so this entrypoint still +# produces a working slapd.conf against a distro OpenLDAP (where the app falls +# back to resolving nesting itself -- see nodejs/models/group_ldap.js). +NESTGROUP_AVAILABLE=0 +if [[ -n "$MODULE_PATH" && -f "$MODULE_PATH/nestgroup.so" ]]; then + NESTGROUP_AVAILABLE=1 +fi + # ── TLS certificate for LDAPS / StartTLS ──────────────────────────────────── # Legacy apps (e.g. the theta42/proxy, Gitea, Emby) bind to LDAP directly over the # network. To keep password binds off the wire in cleartext we expose LDAPS @@ -140,6 +151,7 @@ moduleload ppolicy moduleload memberof moduleload refint moduleload auditlog +NESTGROUP_MODULE_PLACEHOLDER SYNCPROV_MODULE_PLACEHOLDER # TLS (LDAPS on 636 + StartTLS on 389). Cert/key paths are fixed; the files are @@ -188,6 +200,8 @@ memberof-memberof-ad memberOf overlay refint refint_attributes memberOf member manager owner +NESTGROUP_OVERLAY_PLACEHOLDER + # auditlog overlay (LDIF audit trail of all changes) overlay auditlog auditlog /var/lib/ldap/auditlog.ldif @@ -221,6 +235,37 @@ else sed -i "/^SLAPMODULEPATH$/d" /etc/openldap/slapd.conf fi +# ── Nested groups (nestgroup overlay) ── +# Three of the four flags, deliberately: +# +# member-filter (member=X) finds parent groups transitively. This is what +# Group.list(dn) rides on -- the core access question. +# memberof-filter (memberOf=X) matches members of nested groups. This is +# what SSSD's ldap_access_filter uses, so SSH/sudo inherit +# nesting without any client-side walking. +# memberof-values expands memberOf when reading a user, so anything that +# reads the attribute rather than searching still sees the +# full picture. +# +# member-values is deliberately NOT enabled. It expands the `member` attribute +# when reading a *group*, which sounds symmetric but destroys the distinction +# between "listed on this group" and "reachable through a nested one" -- and +# that distinction is not recoverable afterwards, because the raw values are +# simply not returned. The Groups UI needs it to show nested groups as nested +# rather than as a crowd of phantom users, and un-nesting needs it to know what +# it is actually removing. Transitive *answers* come from the filter flags and +# from Group.effectiveMembers(), which computes the closure explicitly. +if [[ "$NESTGROUP_AVAILABLE" == "1" ]]; then + info "nestgroup overlay available — nested groups resolved server-side" + sed -i "s|^NESTGROUP_MODULE_PLACEHOLDER$|moduleload nestgroup|" /etc/openldap/slapd.conf + NESTGROUP_BLOCK="# nestgroup overlay (server-side nested group evaluation)\noverlay nestgroup\nnestgroup-base ou=groups,${LDAP_BASE_DN}\nnestgroup-flags member-filter memberof-filter memberof-values" + sed -i "s|^NESTGROUP_OVERLAY_PLACEHOLDER$|${NESTGROUP_BLOCK}|" /etc/openldap/slapd.conf +else + info "nestgroup overlay not present in ${MODULE_PATH:-} — nested groups will be resolved by the app instead" + sed -i "/^NESTGROUP_MODULE_PLACEHOLDER$/d" /etc/openldap/slapd.conf + sed -i "/^NESTGROUP_OVERLAY_PLACEHOLDER$/d" /etc/openldap/slapd.conf +fi + # ── Multi-Master Replication Configuration ── if [[ -n "${LDAP_SERVER_ID:-}" && -n "${LDAP_REPLICATION_HOSTS:-}" ]]; then info "Configuring Multi-Master replication (Server ID: ${LDAP_SERVER_ID})" @@ -310,7 +355,7 @@ EOF # Required SSO groups. The app gates admin/invite/oauth-admin on these; # app_sso_service_account is a marker (not a permission gate) for # non-person accounts -- see the Users page. - for group in app_sso_admin app_sso_invite app_sso_oauth_admin app_sso_service_account; do + for group in app_super_admin app_sso_admin app_sso_invite app_sso_oauth_admin app_sso_service_account; do ldapadd -x -D "$LDAP_BIND_DN" -w "$LDAP_ADMIN_PASS" -H ldap://localhost:389 << EOF || true dn: cn=${group},ou=groups,${LDAP_BASE_DN} objectClass: groupOfNames @@ -321,6 +366,27 @@ member: ${LDAP_BIND_DN} EOF done + # Nest app_super_admin into the SSO admin groups, so cross-app super admins + # hold those rights by membership rather than by a special case in app code. + # This is what makes the privilege visible to every consumer -- SSSD, sudo, + # anything binding LDAP directly -- instead of only to callers that happen + # to route through utils/permission.js. + # + # app_sso_service_account is deliberately excluded: it is a marker for + # non-person accounts, not a permission, and nesting admins into it would + # misclassify them as service accounts on the Users page. + if [[ "$NESTGROUP_AVAILABLE" == "1" ]]; then + for group in app_sso_admin app_sso_invite app_sso_oauth_admin; do + ldapmodify -x -D "$LDAP_BIND_DN" -w "$LDAP_ADMIN_PASS" -H ldap://localhost:389 >/dev/null 2>&1 << EOF || true +dn: cn=${group},ou=groups,${LDAP_BASE_DN} +changetype: modify +add: member +member: cn=app_super_admin,ou=groups,${LDAP_BASE_DN} +EOF + done + info "Nested app_super_admin into the SSO admin groups" + fi + info "LDAP directory initialized" else info "LDAP directory already initialized — skipping seed" @@ -380,6 +446,14 @@ if [[ "${SECRETS_JS_MODE:-0}" != 1 ]]; then export app_ldap__bindPassword="${app_ldap__bindPassword:-$LDAP_ADMIN_PASS}" export app_ldap__userBase="${app_ldap__userBase:-ou=people,${LDAP_BASE_DN}}" export app_ldap__groupBase="${app_ldap__groupBase:-ou=groups,${LDAP_BASE_DN}}" + # Tell the app whether slapd resolves nested groups for it. When true the app + # trusts a plain (member=) search to be transitive; when false it computes + # the closure itself. Getting this wrong in the "true" direction silently + # under-grants, so it is derived from the same nestgroup.so probe that + # decides whether the overlay is configured at all -- never hardcoded. + if [[ "$NESTGROUP_AVAILABLE" == "1" ]]; then + export app_ldap__nestedGroupsServerSide="${app_ldap__nestedGroupsServerSide:-true}" + fi export app_oauth__jwtSecret="${app_oauth__jwtSecret:-$JWT_SECRET}" # OIDC issuer advertised in /.well-known/openid-configuration. Default to the # public https URL on the SSO subdomain of the LDAP domain; override with diff --git a/docs/_config.yml b/docs/_config.yml index 7c50765..be9e714 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -34,6 +34,11 @@ nav: - title: Directory page: /directory.html icon: fa-server + # API.md lives at the repo root, not under docs/, so Jekyll never renders an + # api.html for it — link the source directly, same as the Changelog. + - title: API + url: https://github.com/theta42/sso-manager-node/blob/master/API.md + icon: fa-code - title: Changelog url: https://github.com/theta42/sso-manager-node/blob/master/CHANGELOG.md icon: fa-list diff --git a/docs/concepts-accounts.md b/docs/concepts-accounts.md index 120be02..e782ab5 100644 --- a/docs/concepts-accounts.md +++ b/docs/concepts-accounts.md @@ -53,6 +53,29 @@ photo server. Once a group exists, add or remove members from the **Groups** page, and point the other app's "who's allowed in" setting at that group's name. +### Groups inside groups + +A group can contain another group, not just people — the *Nested* tab on any +group card. Everyone in the inner group counts as a member of the outer one, +however many levels deep it goes. + +This is mostly a way to stop repeating yourself. Make one `developers` group, +nest it into the handful of things developers should reach, and adding a new +developer to that one group grants all of them at once — instead of adding them +to each individually and slowly drifting out of sync. The app already does this +for itself: super admins are nested into every resource's admin group, and each +admin group into its access group, so "can administer it" always implies "can +use it". + +Two things it won't let you do: put a group inside itself (directly or round a +longer loop), and empty a group completely — every group must keep at least one +member. + +A note if you also manage the directory by hand: a group's member list shows +what is *directly* listed on it. Someone who gets in through a nested group is +a real member but won't appear there — the **Nested** tab shows what is nested, +and the API's `effective` view lists everyone who actually gets in. + ## Every account's personal group Separately from the groups above, every single account — person or diff --git a/docs/concepts-api-tokens.md b/docs/concepts-api-tokens.md index 6540ff8..36eeb9d 100644 --- a/docs/concepts-api-tokens.md +++ b/docs/concepts-api-tokens.md @@ -8,7 +8,7 @@ description: A plain-language guide to personal access tokens in SSO Manager. This page explains what an API token is and when you'd want one. For the full list of API endpoints a token can call, see the -[API reference](api.html). +[API reference](https://github.com/theta42/sso-manager-node/blob/master/API.md). ## What's an API token, in plain terms? @@ -54,6 +54,6 @@ it stops working right away. ## Want more detail? This page doesn't attempt to list every API endpoint or show request/ -response examples — for that, see the full [API reference](api.html). +response examples — for that, see the full [API reference](https://github.com/theta42/sso-manager-node/blob/master/API.md). [← Back to Home](index.html) diff --git a/docs/directory.md b/docs/directory.md index 1a671e7..1287c6c 100644 --- a/docs/directory.md +++ b/docs/directory.md @@ -54,6 +54,28 @@ Resources carry a flexible `metadata` JSON object that can store essential conte - **Install Path**: The filesystem path where the service is installed (e.g. `/opt/app`). - **Systemd Service**: The systemd unit name for the service (e.g. `app.service`). +### Who sees which metadata + +Metadata keys are declared in `@simpleworkjs/directory-schema` with an `admin` flag, and every API response is passed through its projection. There are three tiers: + +- **Public** — returned to any authenticated caller, including machine (`ServiceToken`) callers: `ip`, `address`, `sshPort`, `fqdn`, `dnsNames`, `port`, `externalPort`, `portMappings`, `isExternalReachable`, `os`, `gitRepo`, `subType`, `icon`, `tagline`, `isPublic`, `isProduction`, `requestable`, `isCurrentSite`. +- **Admin-only** — only for members of `app_sso_directory_admin` / `app_sso_admin`: `vmid`, `macAddress`, `installPath`, `systemdService`, and the OAuth config keys (`redirect_uris`, `scopes`, `allowed_groups`, `token_lifetime`). +- **Never returned** — `client_secret_hash`, plus any key matching `/secret|password|privatekey/i`. Stripped on every path, admins included. + +Note that machine tokens are deliberately *not* admins, so anything a machine consumer needs (the firewall generator reads `port` / `externalPort` / `isExternalReachable`) has to be in the public tier. A metadata key that isn't declared at all is treated as admin-only and will silently vanish for normal users — if you add a field to the admin form, declare it in the schema package too. + +## Catalog & access requests + +The site root (`/`) is the end-user catalog — the only ungated page in the nav. It shows: + +- **My Access** — everything the signed-in user can reach (`GET /api/discovery/me`), each card carrying a **how to reach it** block: the URL for a service, or the SSH invocation for a host. When `directory.jumpHost` is set in the config, host cards render the jump-host form `ssh _-_@`; otherwise they fall back to a direct `ssh @`. +- **Discover More** — everything else in the directory, with a **Request access** button. +- **My Requests** / **Awaiting My Approval** — pending requests, and the approve/deny queue for anyone who owns a requested resource. + +A request is a proposal to join an LDAP group. It targets the resource's `member`-level group (the `_access` one, never `_admin`), and approving it performs the LDAP group add — so LDAP stays the single access-control truth and the table is just the audit trail. Approvals are idempotent: approving for someone already in the group succeeds rather than erroring. + +Requests are decided by the resource's `owner`, or by any directory admin. Mark a resource `metadata.requestable = false` to keep it out of self-service. + ## Navigating the UI The Directory Management interface provides a **Tree View** toggle that visually nests your resources, making it easy to comprehend your network topography at a glance. You can also filter, search, and sort your entire infrastructure inventory. From the tree view, you can click the green `+` icon next to any resource to instantly add a child resource beneath it. @@ -104,4 +126,14 @@ All of the above uses the same admin API the UI does (group `app_sso_directory_a - `GET/POST /api/directory-admin/resources`, `PUT/DELETE /api/directory-admin/resources/:id` - `GET/POST/DELETE /api/directory-admin/edges` — parent/child links (`hosts`, `oauth` relations) - `GET/POST/DELETE /api/directory-admin/groups` — resource ↔ LDAP group links +- `GET /api/directory-admin/access-summary` — per-resource group + member counts (the Access column) +- `GET /api/directory-admin/user-access/:uid` — the reverse lookup: every resource a given user can reach, and via which group - Read-only graph views (any authenticated user): `GET /api/discovery/resources`, `/api/discovery/resources/:slug`, `/api/discovery/graph`, `/api/discovery/me` + +Access requests are open to any authenticated user; deciding is gated per-resource inside the router (resource owner or directory admin): + +- `POST /api/access-requests` — `{slug | resourceId, groupCn?, note?}` +- `GET /api/access-requests/mine` — the caller's own history +- `GET /api/access-requests` — pending requests the caller may decide +- `POST /api/access-requests/:id/approve` · `POST /api/access-requests/:id/deny` +- `DELETE /api/access-requests/:id` — the requester withdraws their own pending request diff --git a/docs/ldap.md b/docs/ldap.md index 8cdd785..dd405bb 100644 --- a/docs/ldap.md +++ b/docs/ldap.md @@ -59,8 +59,41 @@ way or use `slappasswd -h '{SSHA512}'`. Groups are `cn=,ou=groups,` (`groupOfNames`) with a `member` attribute listing member DNs. The `memberOf` overlay populates reverse membership (`memberOf` on the user); `refint` keeps it consistent on -add/remove. **Admin permission checks read the group's `member` list**, not -`memberOf` on the user. +add/remove. + +Note that `groupOfNames` requires **at least one member**, which has two +consequences worth knowing: whoever creates a group is automatically seeded +into it, and removing the last member (user *or* nested group) is refused with +a 409 rather than leaving an invalid entry behind. + +### Nested groups + +A `member` DN may be another group's, not just a user's — that is how nesting +is stored, with no extra schema. Everyone in the nested group is a member of +the outer one, at any depth. Manage it on the **Groups** page under each +group's *Nested* tab, or via the API: + +``` +PUT /api/group/:group/nested/:child nest :child inside :group +DELETE /api/group/:group/nested/:child un-nest +GET /api/group/:group/effective direct users, nested groups, and the + full transitive set of users +``` + +Cycles are refused (409) rather than truncated — a loop makes "who is in this +group" unanswerable. Two standing relationships are wired automatically: the +cross-app `app_super_admin` is nested into every resource's `_admin` +group, and each `_admin` into its `_access` group, so administering +something implies being able to use it. + +**Resolving nesting is a client-side job on stock OpenLDAP.** No 2.6.x release +can evaluate nested groups; `memberOf` and a `(member=X)` filter both return +direct membership only. The bundled slapd is therefore built from source with +the `nestgroup` overlay (see *Modules + overlays* below), and the app is told so +via `ldap.nestedGroupsServerSide`. Against any other server the app computes the +closure itself — same answers, more queries. Either way, **never read `memberOf` +directly to make an access decision**; use `utils/user_groups.js`'s `groupCns()`, +which is correct in both modes. ### Personal groups @@ -75,15 +108,15 @@ instead from the owning user's own profile page ("Members of ``'s group", admin-only) — add other accounts as supplementary members, e.g. to share write access to files owned by this group. -The SSO requires three groups (seeded automatically by the entrypoint / -`install.sh`): +The SSO seeds these groups automatically (entrypoint / `install.sh`): | Group | Grants | |-------|--------| +| `app_super_admin` | cross-app super admin. Nested into the three below, so its members hold those rights transitively rather than by a special case in app code — and the privilege is visible to LDAP-native consumers (SSSD, sudo) too. | | `app_sso_admin` | full admin (users, groups, settings) | | `app_sso_oauth_admin` | OAuth client management | | `app_sso_invite` | invitation management | -| `app_sso_service_account` | not a permission — marks a `posixAccount` as a non-person service account (see *Service accounts* below) | +| `app_sso_service_account` | not a permission — marks a `posixAccount` as a non-person service account (see *Service accounts* below). Deliberately **not** nested into, since it changes how an account is displayed rather than what it may do. | ## TLS (LDAPS / StartTLS) @@ -308,11 +341,37 @@ needs: - **Modules:** `pw-sha2` (the app stores user passwords as `{SSHA512}`), `ppolicy`, `memberof`, `refint`. +- **Optional — `nestgroup`:** server-side nested-group evaluation. Not in any + released OpenLDAP (added to master as ITS#10161 in March 2024; 2.7 is still + unreleased), so the bundled image builds slapd from a pinned upstream commit. + Without it the app resolves nesting itself and everything still works — leave + `ldap.nestedGroupsServerSide` at `false`. With it, set that to `true` and + configure: + + ``` + overlay nestgroup + nestgroup-base ou=groups, + nestgroup-flags member-filter memberof-filter memberof-values + ``` + + Flags are **space-separated**; the comma form the man page's `{a, b, c}` + notation suggests is rejected. `member-values` is deliberately omitted — it + expands the `member` attribute when reading a group, which destroys the + distinction between "listed here" and "reachable through a nested group", and + the raw values are then unrecoverable. Transitive answers come from the filter + flags and from `GET /api/group/:group/effective`. + + One more consequence of building from master: it ships **LMDB 1.0.0**, whose + on-disk format is mutually unreadable with the 0.9.x in 2.6.x + (`MDB_INVALID: File is not an LMDB file`). Moving a directory between the two + is a `slapcat` → `slapadd` reload, not a restart. - **Custom schema:** the `theta42Person` auxiliary objectClass with `dateOfBirth` — see `ops/ldap-setup.sh` for the LDIF. - **Directory tree:** `ou=people`, `ou=groups`, `ou=policies` under the base DN, a default `pwdPolicy` at `cn=ppolicy,ou=policies,`. -- **Required groups:** `app_sso_admin`, `app_sso_invite`, `app_sso_oauth_admin`. +- **Required groups:** `app_sso_admin`, `app_sso_invite`, `app_sso_oauth_admin`, + and `app_super_admin` (the cross-app super-admin group; the bundled entrypoint + also nests it into the first three). `ops/ldap-setup.sh -p ` configures all of the above idempotently against a running slapd (auto-detects the database holding your diff --git a/nodejs/app.js b/nodejs/app.js index faeb43a..6e9a1af 100755 --- a/nodejs/app.js +++ b/nodejs/app.js @@ -91,6 +91,9 @@ app.use('/api/group', middleware.auth, require('./routes/group')); app.use('/api/notification', middleware.auth, require('./routes/notification')); app.use('/api/discovery', middleware.auth, require('./routes/discovery')); app.use('/api/directory-admin', middleware.auth, require('./routes/api_directory_admin')); +// Self-service access requests — any authenticated user may ask; deciding is +// gated per-resource inside the router (owner or directory admin). +app.use('/api/access-requests', middleware.auth, require('./routes/access_request')); app.use('/api/update-check', middleware.auth, require('./routes/update_check')); app.use('/api/tos', middleware.auth, require('./routes/tos')); app.use('/api/metrics', middleware.auth, require('./routes/api_metrics')); diff --git a/nodejs/conf/base.js b/nodejs/conf/base.js index f76bd3e..ed989a1 100644 --- a/nodejs/conf/base.js +++ b/nodejs/conf/base.js @@ -29,6 +29,11 @@ module.exports = { // public 636 port forward. See docs/ldap.md. ldapsHost: '', ldapsPort: 636, + // True when slapd carries the `nestgroup` overlay, which resolves nested + // groups server-side. Set automatically by docker-entrypoint.sh for the + // all-in-one image; leave false when pointing at a stock OpenLDAP (no + // 2.6.x release ships nestgroup) and the app resolves nesting itself. + nestedGroupsServerSide: false, // New users/personal groups (see addPosixAccount/addPosixGroup in // models/user_ldap.js) get the next uid/gidNumber >= uidGidMin. // Existing entries >= uidGidReservedFloor are ignored when computing @@ -60,6 +65,16 @@ module.exports = { pass: '__in secrets file__', from: 'SSO Manager ', }, + directory: { + // Public SSH jump host fronting the lab, if there is one (the jump-host + // component). When set, a host card in the catalog shows the real + // invocation — `ssh _-_@` — instead of a bare + // `ssh @` that only works from inside the LAN. Empty is fine; + // the card falls back to the direct form. + jumpHost: '', + // Default SSH port assumed when a host carries no metadata.sshPort. + defaultSshPort: 22, + }, service: { updateCheck: { enabled: true, diff --git a/nodejs/models/access_request.js b/nodejs/models/access_request.js new file mode 100644 index 0000000..1a91294 --- /dev/null +++ b/nodejs/models/access_request.js @@ -0,0 +1,57 @@ +'use strict'; + +// Self-service access requests: the "request" half of the directory catalog. +// +// A request is a *proposal to join an LDAP group*. Approving one does exactly +// what an admin would have done by hand -- add the user to `groupCn` -- so LDAP +// remains the single access-control truth and this table is only the paper +// trail of who asked, who decided, and when. Nothing here grants anything on +// its own; a row with status 'approved' whose LDAP write failed is a row that +// grants no access, which is the safe direction. + +const { Model } = require('@simpleworkjs/orm'); + +const STATUS = { + PENDING: 'pending', + APPROVED: 'approved', + DENIED: 'denied', + CANCELLED: 'cancelled', +}; + +class AccessRequest extends Model { + static fields = { + id: { type: 'uuid', primaryKey: true }, + // The requesting user's uid (not dn): dn changes if the directory is + // restructured, uid is the stable handle used everywhere else in the app. + uid: { type: 'string', isRequired: true }, + resource: { type: 'hasOne', model: 'Resource' }, // creates resourceId + // The group joining which satisfies this request. Captured at request time + // so a later re-link of the resource's groups can't silently redirect a + // pending approval at a different group than the one that was reviewed. + groupCn: { type: 'string', isRequired: true }, + status: { type: 'string', isRequired: true, default: STATUS.PENDING }, + note: { type: 'text' }, + requestedOn: { type: 'integer' }, + decidedBy: { type: 'string' }, + decidedOn: { type: 'integer' }, + decisionNote: { type: 'text' }, + }; + + // The one request that blocks a new one: same user, same group, still open. + // Denied/cancelled requests deliberately do not block -- circumstances change + // and a user may ask again. + static async findOpen(uid, groupCn) { + const rows = await this.list({ where: { uid, groupCn, status: STATUS.PENDING } }); + return rows[0] || null; + } + + static async listForUser(uid) { + return this.list({ where: { uid } }); + } + + static async listPending() { + return this.list({ where: { status: STATUS.PENDING } }); + } +} + +module.exports = { AccessRequest, STATUS }; diff --git a/nodejs/models/group_ldap.js b/nodejs/models/group_ldap.js index 92722cc..70c12e7 100644 --- a/nodejs/models/group_ldap.js +++ b/nodejs/models/group_ldap.js @@ -112,18 +112,190 @@ async function cachedListDetail() { return promise; } +// --- Nested groups ------------------------------------------------------- +// +// `groupOfNames.member` holds DNs, and nothing says those DNs must be users -- +// a group DN is a perfectly legal member. That is how nesting is stored here: +// as-is, no extra schema, no denormalization, the nesting visible in LDAP +// exactly as an admin entered it. +// +// What LDAP will NOT do is resolve it. The memberof overlay records only +// *direct* membership, and a `(member=)` filter likewise finds only the +// groups that list the DN literally. So transitivity is computed here, and +// every membership question in the app must go through these helpers or it +// will silently see one level and grant nothing for a nested group. +// +// The whole group set is one subtree search, so the closure is computed in +// memory rather than issuing a query per level. `resolverCache` keeps that +// search off the hot path for bursts; it is cleared by every write below, so +// the only staleness it can introduce is from edits made outside this app. +// Auth decisions ride on this, hence the deliberately short TTL. + +const NESTING_TTL_MS = 15 * 1000; +const MAX_NESTING_DEPTH = Number(conf.groupNestingDepth) > 0 ? Number(conf.groupNestingDepth) : 10; + +const resolverCache = new LRUCache({ max: 1, ttl: NESTING_TTL_MS, ttlAutopurge: true }); + +async function allGroupsForResolver() { + const hit = resolverCache.get('all'); + if (hit) return hit; + const promise = withClient(async (client) => { + const groups = await getGroups(client); + return groups.map(g => ({ ...g })); + }).then(plain => { + resolverCache.set('all', plain); + return plain; + }).catch(err => { + resolverCache.delete('all'); + throw err; + }); + resolverCache.set('all', promise); + return promise; +} + +const lc = dn => String(dn || '').toLowerCase(); + +// dn -> [groups that list dn as a member]. One pass, reused for every lookup. +function buildParentIndex(groups) { + const parents = new Map(); + for (const group of groups) { + for (const member of [].concat(group.member || []).filter(Boolean)) { + const key = lc(member); + if (!parents.has(key)) parents.set(key, []); + parents.get(key).push(group); + } + } + return parents; +} + +// Every group `dn` belongs to, directly or through any chain of nested groups. +// Breadth-first with a visited set, so a cycle (A in B, B in A) terminates +// instead of hanging, and MAX_NESTING_DEPTH bounds a pathological chain. +function closureUp(dn, groups) { + const parents = buildParentIndex(groups); + const found = new Map(); // cn -> group + const seen = new Set([lc(dn)]); + let frontier = [lc(dn)]; + + for (let depth = 0; depth < MAX_NESTING_DEPTH && frontier.length; depth++) { + const next = []; + for (const current of frontier) { + for (const group of parents.get(current) || []) { + const groupDn = lc(group.dn); + if (seen.has(groupDn)) continue; + seen.add(groupDn); + found.set(group.cn, group); + // The group itself is now a member to look up: this is the step + // that makes the walk transitive rather than one-level. + next.push(groupDn); + } + } + frontier = next; + } + return [...found.values()]; +} + +// Every member DN reachable from a group, split into the users it effectively +// grants and the groups it nests. `direct` is kept separate so the UI can show +// "3 members, 12 effective" and so removal stays unambiguous. +function closureDown(group, groups) { + const byDn = new Map(groups.map(g => [lc(g.dn), g])); + const users = new Set(); + const nested = new Map(); + const seen = new Set([lc(group.dn)]); + let frontier = [group]; + + for (let depth = 0; depth < MAX_NESTING_DEPTH && frontier.length; depth++) { + const next = []; + for (const current of frontier) { + for (const member of [].concat(current.member || []).filter(Boolean)) { + const key = lc(member); + const asGroup = byDn.get(key); + if (asGroup) { + if (seen.has(key)) continue; + seen.add(key); + nested.set(asGroup.cn, asGroup); + next.push(asGroup); + } else { + users.add(member); + } + } + } + frontier = next; + } + return { users: [...users], nested: [...nested.values()] }; +} + var Group = {}; +// Set when slapd carries the nestgroup overlay (docker-entrypoint.sh exports +// app_ldap__nestedGroupsServerSide=true after detecting nestgroup.so). With it, +// a plain `(member=)` search already returns the full transitive set and the +// in-app closure is redundant work on every request. Without it -- e.g. pointed +// at a stock 2.6.x server, which no release ships nestgroup in -- the app must +// compute the closure itself or nested groups silently grant nothing. +const SERVER_SIDE_NESTING = String(conf.nestedGroupsServerSide) === 'true'; + +// Transitive: every group CN this member belongs to, at any nesting depth. +// Callers making an access decision must use this rather than reading +// `memberOf`, which a server without nestgroup only ever populates one level +// deep. Group.list = async function(member){ if (member) { - return withClient(async (client) => { - const groups = await getGroups(client, member); - return groups.map(group => group.cn); - }); + if (SERVER_SIDE_NESTING) { + return withClient(async (client) => { + const groups = await getGroups(client, member); + return groups.map(group => group.cn); + }); + } + const groups = await allGroupsForResolver(); + return closureUp(member, groups).map(group => group.cn); } return (await cachedListDetail()).map(group => group.cn); } +// The members a group effectively grants: users reached through any chain of +// nested groups, plus the nested groups themselves for display. +Group.effectiveMembers = async function(cn){ + const groups = await allGroupsForResolver(); + const group = groups.find(g => g.cn === cn); + if (!group) { + let error = new Error('GroupNotFound'); + error.name = 'GroupNotFound'; + error.message = `LDAP:${cn} does not exists`; + error.status = 404; + throw error; + } + const { users, nested } = closureDown(group, groups); + const directMembers = [].concat(group.member || []).filter(Boolean); + const groupDns = new Set(groups.map(g => lc(g.dn))); + return { + cn: group.cn, + direct: directMembers.filter(dn => !groupDns.has(lc(dn))), + nestedGroups: nested.map(g => ({ cn: g.cn, dn: g.dn })), + effective: users, + }; +}; + +// Would adding `childDn` to `parentCn` create a cycle? A group may not contain +// itself, nor anything that already (transitively) contains it -- such a chain +// makes membership unanswerable, and callers would rely on the depth cap to +// stop rather than getting a real answer. +Group.wouldCycle = async function(parentCn, childDn){ + const groups = await allGroupsForResolver(); + const parent = groups.find(g => g.cn === parentCn); + if (!parent) return false; + if (lc(parent.dn) === lc(childDn)) return true; + const child = groups.find(g => lc(g.dn) === lc(childDn)); + if (!child) return false; // a user DN can never close a cycle + // Adding child under parent is a cycle exactly when parent is already + // reachable downward from child. + const { nested } = closureDown(child, groups); + return nested.some(g => lc(g.dn) === lc(parent.dn)); +}; + +Group.clearResolverCache = function(){ resolverCache.clear(); }; + Group.listDetail = async function(member){ if (member) { return withClient(async (client) => getGroups(client, member)); @@ -166,6 +338,7 @@ Group.add = async function(data){ return withClient(async (client) => { await addGroup(client, data); cache.clear(); + resolverCache.clear(); return this.get(data); }); } @@ -174,6 +347,7 @@ Group.addMember = async function(user){ await withClient(async (client) => addMember(client, this, user)); this.member = [].concat(this.member || []).concat([user.dn]); cache.clear(); + resolverCache.clear(); return this; }; @@ -186,6 +360,7 @@ Group.removeMember = async function(user){ } this.member = [].concat(this.member || []).filter(dn => dn !== user.dn); cache.clear(); + resolverCache.clear(); return this; }; @@ -193,6 +368,7 @@ Group.addOwner = async function(user){ await withClient(async (client) => addOwner(client, this, user)); this.owner = [].concat(this.owner || []).concat([user.dn]); cache.clear(); + resolverCache.clear(); return this; }; @@ -205,12 +381,14 @@ Group.removeOwner = async function(user){ } this.owner = [].concat(this.owner || []).filter(dn => dn !== user.dn); cache.clear(); + resolverCache.clear(); return this; }; Group.remove = async function(){ await withClient(async (client) => client.del(this.dn)); cache.clear(); + resolverCache.clear(); return true; } diff --git a/nodejs/models/index.js b/nodejs/models/index.js index 49b5110..18db4b0 100644 --- a/nodejs/models/index.js +++ b/nodejs/models/index.js @@ -14,6 +14,7 @@ require('./api_token'); const { init } = require('@simpleworkjs/orm'); const { Resource, ResourceEdge, ResourceGroup } = require('./resource'); +const { AccessRequest } = require('./access_request'); async function initORM() { const ormConf = conf.orm || { @@ -28,7 +29,7 @@ async function initORM() { await init({ conf: { orm: ormConf }, models: [ - Resource, ResourceEdge, ResourceGroup, + Resource, ResourceEdge, ResourceGroup, AccessRequest, Token, AuthToken, InviteToken, ImpersonationToken, PasswordResetToken, OtpToken, ServiceToken ] }); diff --git a/nodejs/models/resource.js b/nodejs/models/resource.js index 399a4ad..42d1998 100644 --- a/nodejs/models/resource.js +++ b/nodejs/models/resource.js @@ -103,6 +103,40 @@ class Resource extends Model { return { resources: resObjs, edges }; } + // Stamp `resolvedAddress` on each resource: its own address/ip if it has one, + // otherwise the nearest ancestor's. A service usually carries no address of + // its own -- it is reached at the host it runs on -- so "how do I reach this" + // is only answerable from the graph, never from the row alone. Every caller + // that answers that question for a user (getMyAccess, GET /api/discovery/me) + // must go through here, or services come back unreachable. + static async withResolvedAddress(resources) { + if (!resources || !resources.length) return []; + const graph = await this.getGraph(); + + const resolve = (resId, visited = new Set()) => { + if (visited.has(resId)) return null; // prevent cycles + visited.add(resId); + + const res = graph.resources.find(r => r.id === resId); + if (!res) return null; + if (res.metadata && res.metadata.address) return res.metadata.address; + if (res.metadata && res.metadata.ip) return res.metadata.ip; + + for (const edge of graph.edges.filter(e => e.childId === resId)) { + const found = resolve(edge.parentId, visited); + if (found) return found; + } + return null; + }; + + return resources.map(r => { + const data = r.toJSON ? r.toJSON() : { ...r }; + data.metadata = data.metadata || {}; + data.resolvedAddress = resolve(data.id); + return data; + }); + } + static async getMyAccess(userDn) { const userGroups = await Group.list(userDn); if (!userGroups || userGroups.length === 0) return []; @@ -110,38 +144,11 @@ class Resource extends Model { const resourceGroups = await ResourceGroup.list({ where: { groupCn: { in: userGroups } } }); - + const resourceIds = [...new Set(resourceGroups.map(rg => rg.resourceId))]; if (resourceIds.length === 0) return []; - - const resources = await this.list({ where: { id: { in: resourceIds } } }); - - // Resolve inherited addresses from the graph - const graph = await this.getGraph(); - - function resolveHost(resId, visited = new Set()) { - if (visited.has(resId)) return null; // prevent cycles - visited.add(resId); - - const res = graph.resources.find(r => r.id === resId); - if (!res) return null; - if (res.metadata && res.metadata.address) return res.metadata.address; - if (res.metadata && res.metadata.ip) return res.metadata.ip; - - const parentEdges = graph.edges.filter(e => e.childId === resId); - for (const edge of parentEdges) { - const found = resolveHost(edge.parentId, visited); - if (found) return found; - } - return null; - } - - return resources.map(r => { - const data = { ...r }; - data.metadata = data.metadata || {}; - data.resolvedAddress = resolveHost(r.id); - return data; - }); + + return this.withResolvedAddress(await this.list({ where: { id: { in: resourceIds } } })); } static fields = { diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index c4a5ea0..21ef99d 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,19 +1,19 @@ { "name": "t42-sso-manager", - "version": "1.8.3", + "version": "1.11.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "t42-sso-manager", - "version": "1.8.3", + "version": "1.11.0", "license": "MIT", "dependencies": { "@fortawesome/fontawesome-free": "^7.3.0", "@popperjs/core": "^2.11.8", "@simpleworkjs/app-stack": "^1.0.0", "@simpleworkjs/conf": "^1.2.0", - "@simpleworkjs/directory-schema": "^1.0.0", + "@simpleworkjs/directory-schema": "^1.1.0", "@simpleworkjs/frontend": "^0.2.7", "@simpleworkjs/ldap": "^1.0.0", "@simpleworkjs/orm": "^0.2.8", @@ -1271,9 +1271,9 @@ } }, "node_modules/@simpleworkjs/directory-schema": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/@simpleworkjs/directory-schema/-/directory-schema-1.0.0.tgz", - "integrity": "sha512-thZhPGNdDYlD8rlhXidnbCHTKjdSkj9ag1zE/gz1AwuclYypsKAP+v3BAvcZ/YDQP8RBDJNPXof5EpVheLovTg==", + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@simpleworkjs/directory-schema/-/directory-schema-1.1.0.tgz", + "integrity": "sha512-hTXxHl7Jz5IbIAYmn8dv9f0B50ocjEg5ju+UV8ZQSaBjJYpetOVfcFvT6v9xwMVtjXSYMDwKPvD8YgKOBz7xJw==", "license": "MIT", "engines": { "node": ">=18.0.0" diff --git a/nodejs/package.json b/nodejs/package.json index 142d175..79dada2 100755 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -1,6 +1,6 @@ { "name": "t42-sso-manager", - "version": "1.10.0", + "version": "1.11.0", "description": "A very simple LDAP management and SSO system", "author": [ { @@ -25,7 +25,7 @@ "@popperjs/core": "^2.11.8", "@simpleworkjs/app-stack": "^1.0.0", "@simpleworkjs/conf": "^1.2.0", - "@simpleworkjs/directory-schema": "^1.0.0", + "@simpleworkjs/directory-schema": "^1.1.0", "@simpleworkjs/frontend": "^0.2.7", "@simpleworkjs/ldap": "^1.0.0", "@simpleworkjs/orm": "^0.2.8", diff --git a/nodejs/routes/access_request.js b/nodejs/routes/access_request.js new file mode 100644 index 0000000..d2f61ca --- /dev/null +++ b/nodejs/routes/access_request.js @@ -0,0 +1,266 @@ +'use strict'; + +// Self-service access requests. Mounted at /api/access-requests (app.js). +// +// The loop this closes: a user browses the catalog, finds something they cannot +// reach, asks for it; the resource's owner (or a directory admin) approves; the +// approval performs the LDAP group add. LDAP stays the access-control truth -- +// this router never invents a permission, it only automates the group add an +// admin would otherwise do by hand, and records who decided. + +const router = require('express').Router(); +const { Resource, ResourceGroup } = require('../models/resource'); +const { AccessRequest, STATUS } = require('../models/access_request'); +const { Group } = require('../models/group_ldap'); +const { User } = require('../models/user_ldap'); +const { Mail } = require('../models/email'); +const { groupCns } = require('../utils/user_groups'); +const { envelope, projectResource } = require('@simpleworkjs/directory-schema'); + +const DIRECTORY_ADMIN_GROUPS = ['app_sso_directory_admin', 'app_sso_admin', 'app_super_admin']; + +function httpError(status, message) { + const err = new Error(message); + err.status = status; + return err; +} + +// May `user` decide requests against `resource`? The resource's own owner is +// the primary approver -- that is the point of Resource.owner -- with directory +// admins as the catch-all so an unowned or orphaned resource is never stuck. +async function canDecide(user, resource, callerGroups) { + if (resource && resource.owner && resource.owner === user.uid) return true; + return callerGroups.some(g => DIRECTORY_ADMIN_GROUPS.includes(g)); +} + +// The group that satisfies a request for this resource. Prefers an explicit +// choice, else the `member`-level link (the "just let me use it" group) over an +// `owner`-level one -- requesting a resource should never silently escalate to +// its admin group. +async function resolveGroupCn(resourceId, requested) { + const links = await ResourceGroup.list({ where: { resourceId } }); + if (!links.length) { + throw httpError(409, 'This resource has no access group linked, so it cannot be requested.'); + } + if (requested) { + const match = links.find(l => l.groupCn === requested); + if (!match) throw httpError(400, `"${requested}" is not an access group for this resource.`); + return match.groupCn; + } + const member = links.find(l => l.accessLevel === 'member'); + return (member || links[0]).groupCn; +} + +// Best-effort notification. A mail failure must never fail the request itself -- +// the row is the source of truth and the approver can find it in the UI. +async function notify(uid, subject, message) { + try { + const user = await User.get({ uid }); + if (!user || !user.mail) return; + await Mail.sendTemplate(user.mail, 'notification', { + givenName: user.givenName || uid, + subject, + message, + }); + } catch (err) { + console.error(`access-request: notification to ${uid} failed:`, err.message); + } +} + +// POST /api/access-requests { slug | resourceId, groupCn?, note? } +router.post('/', async (req, res, next) => { + try { + if (req.user.isMachine) throw httpError(403, 'Machine accounts cannot request access.'); + + let resource; + if (req.body.slug) { + const found = await Resource.list({ where: { slug: req.body.slug } }); + resource = found[0]; + } else if (req.body.resourceId) { + resource = await Resource.get(req.body.resourceId); + } + if (!resource) throw httpError(404, 'Resource not found'); + + const md = resource.metadata || {}; + // Opt-out, not opt-in: everything in the catalog is requestable unless an + // admin has explicitly marked it otherwise. + if (md.requestable === false) { + throw httpError(409, 'This resource is not available for self-service requests.'); + } + + const groupCn = await resolveGroupCn(resource.id, req.body.groupCn); + + const callerGroups = await groupCns(req.user); + if (callerGroups.includes(groupCn)) { + throw httpError(409, 'You already have access to this resource.'); + } + + const existing = await AccessRequest.findOpen(req.user.uid, groupCn); + if (existing) throw httpError(409, 'You already have a pending request for this resource.'); + + const request = await AccessRequest.create({ + uid: req.user.uid, + resourceId: resource.id, + groupCn, + status: STATUS.PENDING, + note: req.body.note || '', + requestedOn: Date.now(), + }); + + if (resource.owner) { + await notify( + resource.owner, + `Access request: ${resource.name}`, + `

${req.user.uid} has requested access to ${resource.name} (group ${groupCn}).

` + + (req.body.note ? `

Their note: ${req.body.note}

` : '') + + `

Review it on the Directory page.

` + ); + } + + res.json(envelope(request)); + } catch (err) { next(err); } +}); + +// GET /api/access-requests/mine — the caller's own request history. +router.get('/mine', async (req, res, next) => { + try { + const rows = await AccessRequest.listForUser(req.user.uid); + res.json(envelope(await decorate(rows))); + } catch (err) { next(err); } +}); + +// GET /api/access-requests — pending requests the caller may decide. +router.get('/', async (req, res, next) => { + try { + const callerGroups = await groupCns(req.user); + const isAdmin = callerGroups.some(g => DIRECTORY_ADMIN_GROUPS.includes(g)); + const pending = await AccessRequest.listPending(); + + let visible = pending; + if (!isAdmin) { + // A plain resource owner sees only requests against resources they own. + const owned = await Resource.list({ where: { owner: req.user.uid } }); + const ownedIds = new Set(owned.map(r => r.id)); + visible = pending.filter(r => ownedIds.has(r.resourceId)); + } + res.json(envelope(await decorate(visible))); + } catch (err) { next(err); } +}); + +// Attach the resource name/slug each row refers to. The UI needs it on every +// list and would otherwise issue one lookup per row. +async function decorate(rows) { + if (!rows.length) return []; + const resources = await Resource.list(); + const byId = new Map(resources.map(r => [r.id, r])); + return rows.map(row => { + const data = row.toJSON ? row.toJSON() : { ...row }; + const resource = byId.get(data.resourceId); + data.resource = resource + ? { id: resource.id, name: resource.name, slug: resource.slug, kind: resource.kind } + : null; + return data; + }); +} + +// POST /api/access-requests/:id/approve { decisionNote? } +router.post('/:id/approve', async (req, res, next) => { + try { + const request = await AccessRequest.get(req.params.id); + if (!request) throw httpError(404, 'Request not found'); + if (request.status !== STATUS.PENDING) { + throw httpError(409, `This request was already ${request.status}.`); + } + + const resource = await Resource.get(request.resourceId); + const callerGroups = await groupCns(req.user); + if (!(await canDecide(req.user, resource, callerGroups))) { + throw httpError(403, 'You do not have permission to decide this request.'); + } + + // The LDAP write happens FIRST and is allowed to throw. Marking a request + // approved without the group add would show the user a grant they do not + // actually have -- a pending row is recoverable, a lying one is not. + const group = await Group.get(request.groupCn); + const user = await User.get({ uid: request.uid }); + try { + await group.addMember(user); + } catch (err) { + // "already a member" is the goal state, not a failure. This happens + // routinely: groupOfNames requires at least one member, so creating a + // resource seeds its auto-created groups with the creator's DN, and an + // admin may also grant access by hand while a request sits pending. + // Without this the request would 500 and stay pending forever. + const alreadyMember = err.name === 'TypeOrValueExistsError' || err.code === 20; + if (!alreadyMember) throw err; + } + User.clearCache(); // membership feeds cached isAdmin / group-gated nav + + const updated = await request.update({ + status: STATUS.APPROVED, + decidedBy: req.user.uid, + decidedOn: Date.now(), + decisionNote: req.body.decisionNote || '', + }); + + await notify( + request.uid, + `Access approved: ${resource ? resource.name : request.groupCn}`, + `

Your request for ${resource ? resource.name : request.groupCn} was approved by ${req.user.uid}.

` + + `

You may need to sign out and back in for the change to take effect everywhere.

` + ); + + res.json(envelope(updated)); + } catch (err) { next(err); } +}); + +// POST /api/access-requests/:id/deny { decisionNote? } +router.post('/:id/deny', async (req, res, next) => { + try { + const request = await AccessRequest.get(req.params.id); + if (!request) throw httpError(404, 'Request not found'); + if (request.status !== STATUS.PENDING) { + throw httpError(409, `This request was already ${request.status}.`); + } + + const resource = await Resource.get(request.resourceId); + const callerGroups = await groupCns(req.user); + if (!(await canDecide(req.user, resource, callerGroups))) { + throw httpError(403, 'You do not have permission to decide this request.'); + } + + const updated = await request.update({ + status: STATUS.DENIED, + decidedBy: req.user.uid, + decidedOn: Date.now(), + decisionNote: req.body.decisionNote || '', + }); + + await notify( + request.uid, + `Access request declined: ${resource ? resource.name : request.groupCn}`, + `

Your request for ${resource ? resource.name : request.groupCn} was declined.

` + + (req.body.decisionNote ? `

Reason: ${req.body.decisionNote}

` : '') + ); + + res.json(envelope(updated)); + } catch (err) { next(err); } +}); + +// DELETE /api/access-requests/:id — requester withdraws their own pending request. +router.delete('/:id', async (req, res, next) => { + try { + const request = await AccessRequest.get(req.params.id); + if (!request) throw httpError(404, 'Request not found'); + if (request.uid !== req.user.uid) { + throw httpError(403, 'You can only withdraw your own requests.'); + } + if (request.status !== STATUS.PENDING) { + throw httpError(409, `This request was already ${request.status}.`); + } + const updated = await request.update({ status: STATUS.CANCELLED, decidedOn: Date.now() }); + res.json(envelope(updated)); + } catch (err) { next(err); } +}); + +module.exports = router; diff --git a/nodejs/routes/api_directory_admin.js b/nodejs/routes/api_directory_admin.js index c70bd0e..d0eb5ed 100644 --- a/nodejs/routes/api_directory_admin.js +++ b/nodejs/routes/api_directory_admin.js @@ -3,8 +3,32 @@ const router = require('express').Router(); const permission = require('../utils/permission'); const { Resource, ResourceEdge, ResourceGroup } = require('../models/resource'); const { Group } = require('../models/group_ldap'); +const { User } = require('../models/user_ldap'); +const { cnFromDn } = require('../utils/user_groups'); const { projectResources } = require('@simpleworkjs/directory-schema'); +const SUPER_ADMIN_GROUP = permission.SUPER_ADMIN_GROUP; + +// Make `childCn` a member of `parentCn`, i.e. everyone in the child is +// transitively in the parent. Idempotent and non-fatal: "already a member" is +// the goal state, and a missing group (e.g. app_super_admin absent on a +// directory seeded by an older entrypoint) is a reason to skip, not to fail the +// caller's real work. +async function nestGroup(childCn, parentCn) { + try { + const parent = await Group.get(parentCn); + const child = await Group.get(childCn); + if (await Group.wouldCycle(parentCn, child.dn)) { + console.error(`nestGroup: refusing ${childCn} -> ${parentCn} (would create a cycle)`); + return; + } + await parent.addMember({ dn: child.dn }); + } catch (err) { + const benign = err.name === 'TypeOrValueExistsError' || err.code === 20 || err.name === 'GroupNotFound'; + if (!benign) console.error(`nestGroup: ${childCn} -> ${parentCn} failed:`, err.message); + } +} + // Require the admin group router.use(async (req, res, next) => { try { @@ -68,8 +92,10 @@ router.post('/resources', async (req, res, next) => { if (r.kind === 'host' || r.kind === 'service') { const siteSlug = await Resource.findAncestorSiteSlug(r.id); + const groupCn = suffix => (siteSlug ? `${siteSlug}_${r.slug}_${suffix}` : `${r.slug}_${suffix}`); + const createGroup = async (suffix, accessLevel) => { - const cn = siteSlug ? `${siteSlug}_${r.slug}_${suffix}` : `${r.slug}_${suffix}`; + const cn = groupCn(suffix); try { await Group.add({ name: cn, @@ -87,6 +113,21 @@ router.post('/resources', async (req, res, next) => { }; await createGroup('access', 'member'); await createGroup('admin', 'owner'); + + // Wire up the two standing relationships every resource has, as nesting + // rather than as membership that has to be maintained per resource: + // + // app_super_admin -> _admin cross-app super admins administer + // every resource, automatically + // _admin -> _access administering something implies + // being able to use it + // + // Before nesting, both of these could only be expressed by adding every + // super admin to every new group by hand -- which nobody does, so the + // groups drifted. A failure here must not fail resource creation: the + // resource and its groups already exist and the nesting is repairable. + await nestGroup(groupCn('admin'), groupCn('access')); + await nestGroup(SUPER_ADMIN_GROUP, groupCn('admin')); } res.json({ results: r }); @@ -103,18 +144,8 @@ router.post('/resources', async (req, res, next) => { router.put('/resources/:id', async (req, res, next) => { try { - let r; - if (req.body.kind === 'oauth') { - const { OAuthClient } = require('../models/oauth_client'); - r = await OAuthClient.get(req.params.id); - } else { - r = await Resource.get(req.params.id); - } - if (!r) return res.status(404).json({ error: 'Not found' }); - - req.body.updated_by = req.user.uid; - req.body.updated_on = Date.now(); - + // Validate before loading anything -- a rejected body should never have + // touched the store. if (req.body.kind === 'host' && !req.body.hostId) { return res.status(400).json({ error: 'Hosts must have a parent Site or Host' }); } @@ -124,14 +155,20 @@ router.put('/resources/:id', async (req, res, next) => { if (req.body.kind === 'oauth' && !req.body.hostId) { return res.status(400).json({ error: 'OAuth Integrations must have a parent Service' }); } - - let updated; - if (req.body.kind === 'oauth') { - updated = await r.update(req.body); - } else { - updated = await r.update(req.body); - } - + + // OAuthClient is a wrapper over the same `resource` row, but its .update() + // handles the oauth-specific body fields (redirect_uris, scopes, + // token_lifetime) that a bare Resource would drop into metadata unvalidated. + const { OAuthClient } = require('../models/oauth_client'); + const model = req.body.kind === 'oauth' ? OAuthClient : Resource; + const r = await model.get(req.params.id); + if (!r) return res.status(404).json({ error: 'Not found' }); + + req.body.updated_by = req.user.uid; + req.body.updated_on = Date.now(); + + const updated = await r.update(req.body); + if ((updated.kind === 'host' || updated.kind === 'service' || updated.kind === 'oauth') && req.body.hostId !== undefined) { const existingEdges = await ResourceEdge.list({ where: { childId: r.id } }); for (const e of existingEdges) { @@ -163,13 +200,18 @@ router.delete('/resources/:id', async (req, res, next) => { try { const r = await Resource.get(req.params.id); if (!r) return res.status(404).json({ error: 'Not found' }); - await r.delete(); - // Also delete edges and groups involving this resource + // Clear the dependents FIRST. There is no transaction here, so ordering is + // the only thing protecting us: if a dependent delete throws after the + // resource row is gone, the leftovers are edges/links pointing at a + // nonexistent id -- invisible in the UI and poisonous to getGraph(). Failing + // with the resource still present is the recoverable direction (retry the + // delete); the caller sees the error either way. const edgesParent = await ResourceEdge.list({ where: { parentId: req.params.id } }); const edgesChild = await ResourceEdge.list({ where: { childId: req.params.id } }); const groups = await ResourceGroup.list({ where: { resourceId: req.params.id } }); for (const e of [...edgesParent, ...edgesChild]) await e.delete(); for (const g of groups) await g.delete(); + await r.delete(); res.json({ results: true }); } catch (err) { next(err); } }); @@ -222,19 +264,138 @@ router.delete('/groups/:id', async (req, res, next) => { } catch (err) { next(err); } }); +// --- Access visibility --- +// +// The two questions an access-control pane has to answer, neither of which the +// directory could answer before: "who can reach this resource" (a column on the +// table, rather than three clicks into a modal) and "what can this user reach" +// (which had no UI at all). Both are joins of the same two sets, so both are +// served from one cached Group.listDetail() rather than a lookup per row. + +// dn -> uid, so member DNs can be reported as the uids admins actually think in. +async function dnToUidMap() { + const users = await User.listDetail(); + return new Map(users.map(u => [String(u.dn).toLowerCase(), u.uid])); +} + +// GET /access-summary — { resourceId: { groups: [...], memberCount } } +router.get('/access-summary', async (req, res, next) => { + try { + const [links, groups, uidByDn] = await Promise.all([ + ResourceGroup.list(), + Group.listDetail(), + dnToUidMap(), + ]); + + const groupByCn = new Map(groups.map(g => [g.cn, g])); + const summary = {}; + + for (const link of links) { + const group = groupByCn.get(link.groupCn); + // A link whose LDAP group has been deleted out from under it: report it + // rather than skipping, since a dangling link grants nothing and the + // admin needs to see that it is dead. + // + // Counts come from the transitive closure, not from `member`. Reading the + // attribute would report only who is listed on the group, missing anyone + // who reaches it through a nested group -- and since app_super_admin is + // nested into every resource's _admin group, that is not an edge case. + let members = []; + if (group) { + const eff = await Group.effectiveMembers(link.groupCn); + members = eff.effective.map(dn => uidByDn.get(String(dn).toLowerCase()) || cnFromDn(dn)); + } + + const entry = summary[link.resourceId] || (summary[link.resourceId] = { groups: [], members: [] }); + entry.groups.push({ + cn: link.groupCn, + accessLevel: link.accessLevel, + exists: !!group, + memberCount: members.length, + }); + for (const uid of members) { + if (!entry.members.includes(uid)) entry.members.push(uid); + } + } + + for (const id of Object.keys(summary)) { + summary[id].memberCount = summary[id].members.length; + } + + res.json({ results: summary }); + } catch (err) { next(err); } +}); + +// GET /user-access/:uid — every resource a given user can reach, and via which +// group. This is the reverse lookup; previously an admin could only see their +// own access, via /api/discovery/me. +router.get('/user-access/:uid', async (req, res, next) => { + try { + const user = await User.get({ uid: req.params.uid }); + if (!user) return res.status(404).json({ error: 'User not found' }); + + const dn = String(user.dn).toLowerCase(); + const groups = await Group.listDetail(); + const memberOf = groups + .filter(g => [].concat(g.member || []).some(m => String(m).toLowerCase() === dn)) + .map(g => g.cn); + + const [links, resources] = await Promise.all([ResourceGroup.list(), Resource.list()]); + const byId = new Map(resources.map(r => [r.id, r])); + + const results = []; + for (const link of links) { + if (!memberOf.includes(link.groupCn)) continue; + const resource = byId.get(link.resourceId); + if (!resource) continue; + results.push({ + id: resource.id, + name: resource.name, + slug: resource.slug, + kind: resource.kind, + groupCn: link.groupCn, + accessLevel: link.accessLevel, + }); + } + + res.json({ results: { uid: user.uid, groups: memberOf, resources: results } }); + } catch (err) { next(err); } +}); + +// Tail the last `lines` lines of a log file without shelling out. Reads at most +// the trailing MAX_TAIL_BYTES so an unrotated multi-GB log can't blow up the +// heap. A missing/unreadable file is normal (the log only exists once slapd has +// written to it), so it yields '' rather than an error. +const MAX_TAIL_BYTES = 256 * 1024; + +async function tailFile(filePath, lines = 100) { + const fs = require('fs/promises'); + let fh; + try { + fh = await fs.open(filePath, 'r'); + const { size } = await fh.stat(); + const start = Math.max(0, size - MAX_TAIL_BYTES); + const buf = Buffer.alloc(Math.min(size, MAX_TAIL_BYTES)); + await fh.read(buf, 0, buf.length, start); + const text = buf.toString('utf8'); + // A partial first line when we started mid-file; drop it. + const rows = (start > 0 ? text.slice(text.indexOf('\n') + 1) : text).split('\n'); + return rows.slice(-lines).join('\n'); + } catch (err) { + return ''; + } finally { + if (fh) await fh.close().catch(() => {}); + } +} + router.get('/audit-logs', async (req, res, next) => { try { - const fs = require('fs'); - const { execSync } = require('child_process'); - let ldapLogs = ''; - let oauthLogs = ''; - let auditLogs = ''; - - try { ldapLogs = execSync('tail -n 100 /var/lib/ldap/slapd.log 2>/dev/null').toString(); } catch(e){} - try { oauthLogs = execSync('tail -n 100 /var/lib/ldap/oauth.log 2>/dev/null').toString(); } catch(e){} - try { auditLogs = execSync('tail -n 100 /var/lib/ldap/auditlog.ldif 2>/dev/null').toString(); } catch(e){} - - res.json({ results: { ldap: ldapLogs, oauth: oauthLogs, audit: auditLogs } }); + const [ldap, oauth, audit] = await Promise.all([ + tailFile('/var/lib/ldap/slapd.log'), + tailFile('/var/lib/ldap/oauth.log'), + tailFile('/var/lib/ldap/auditlog.ldif'), + ]); + res.json({ results: { ldap, oauth, audit } }); } catch (err) { next(err); } }); diff --git a/nodejs/routes/discovery.js b/nodejs/routes/discovery.js index 9acf543..2cb6bfc 100644 --- a/nodejs/routes/discovery.js +++ b/nodejs/routes/discovery.js @@ -10,9 +10,14 @@ // jump-host's `data.results || []` silently collapsed to `[]`, so no user could // bridge) and absorbs the dead /me handler that used to live in // routes/api_discovery.js (mounted after the 404, so unreachable). +// +// Group CNs come from utils/user_groups — `req.user` has `memberOf` (DNs) and +// no `.groups`, so reading `.groups` off it directly yields [] for every human +// caller. See that file for what that silently broke. const router = require('express').Router(); const { Resource, ResourceGroup } = require('../models/resource'); +const { withGroups } = require('../utils/user_groups'); const { envelope, projectResource, @@ -20,20 +25,29 @@ const { isDirectoryAdmin, } = require('@simpleworkjs/directory-schema'); +// Resolve the caller's groups once per request and hand back the projection +// flag. Every handler needs both, and both are wrong if taken off req.user raw. +async function callerView(req) { + const user = await withGroups(req.user); + return { user, fullMetadata: isDirectoryAdmin(user) }; +} + // GET /api/discovery/resources[?kind=&group=&parent=] router.get('/resources', async (req, res, next) => { try { + const { fullMetadata } = await callerView(req); const resources = await Resource.search(req.query); - res.json(envelope(projectResources(resources, { fullMetadata: isDirectoryAdmin(req.user) }))); + res.json(envelope(projectResources(resources, { fullMetadata }))); } catch (err) { next(err); } }); // GET /api/discovery/resources/:slug router.get('/resources/:slug', async (req, res, next) => { try { + const { fullMetadata } = await callerView(req); const resource = await Resource.getBySlug(req.params.slug); // parents/children are edges (no secrets); project only the resource body. - const projected = projectResource(resource, { fullMetadata: isDirectoryAdmin(req.user) }); + const projected = projectResource(resource, { fullMetadata }); projected.parents = resource.parents; projected.children = resource.children; res.json(envelope(projected)); @@ -43,9 +57,10 @@ router.get('/resources/:slug', async (req, res, next) => { // GET /api/discovery/graph router.get('/graph', async (req, res, next) => { try { + const { fullMetadata } = await callerView(req); const graph = await Resource.getGraph(); res.json(envelope({ - resources: projectResources(graph.resources, { fullMetadata: isDirectoryAdmin(req.user) }), + resources: projectResources(graph.resources, { fullMetadata }), edges: graph.edges, })); } catch (err) { next(err); } @@ -54,26 +69,28 @@ router.get('/graph', async (req, res, next) => { // GET /api/discovery/me // Returns the resources the current caller can reach. Machines see only their // own resource; humans get the union of their LDAP groups' resources plus -// anything flagged isPublic. Uses req.user.groups (populated by the auth -// middleware for session/PAT callers) rather than re-querying LDAP by DN, so it -// works for every auth transport without assuming a .dn is present. +// anything flagged isPublic. router.get('/me', async (req, res, next) => { try { + const { user, fullMetadata } = await callerView(req); let accessible; if (req.user && req.user.isMachine) { accessible = await Resource.list({ where: { id: req.resourceId } }); } else { - const userGroups = (req.user && req.user.groups) || []; const ids = new Set(); - if (userGroups.length) { - const rgs = await ResourceGroup.list({ where: { groupCn: { in: userGroups } } }); + if (user.groups.length) { + const rgs = await ResourceGroup.list({ where: { groupCn: { in: user.groups } } }); for (const rg of rgs) ids.add(rg.resourceId); } const all = await Resource.list(); accessible = all.filter(r => ids.has(r.id) || (r.metadata && r.metadata.isPublic)); } - res.json(envelope(projectResources(accessible, { fullMetadata: isDirectoryAdmin(req.user) }))); + // resolvedAddress is the whole point of /me ("how do I reach it") and a + // service inherits it from its host, so it must be computed here rather + // than left to each caller to guess at address || ip. + accessible = await Resource.withResolvedAddress(accessible); + res.json(envelope(projectResources(accessible, { fullMetadata }))); } catch (err) { next(err); } }); -module.exports = router; \ No newline at end of file +module.exports = router; diff --git a/nodejs/routes/group.js b/nodejs/routes/group.js index 9e51bd3..614ad92 100644 --- a/nodejs/routes/group.js +++ b/nodejs/routes/group.js @@ -43,6 +43,87 @@ router.get('/:name', async function(req, res, next){ } }); +// ── Nested groups ─────────────────────────────────────────────────────────── +// A groupOfNames `member` may be any DN, including another group's, which is +// how nesting is stored. These routes are mounted before /:group/:uid so the +// literal "nested"/"effective" path segments are not swallowed by that +// wildcard, which would otherwise try to resolve them as a uid. + +// GET /api/group/:group/effective — who this group actually grants, split into +// directly-listed users, the groups nested into it, and the full transitive set +// of users. The UI shows "3 direct, 12 effective"; a plain member read cannot +// answer that, and on a server with nestgroup it silently returns the expanded +// list with no indication which entries are direct. +router.get('/:group/effective', async function(req, res, next){ + try{ + return res.json({ results: await Group.effectiveMembers(req.params.group) }); + }catch(error){ + next(error); + } +}); + +// PUT /api/group/:group/nested/:child — nest :child inside :group. +router.put('/:group/nested/:child', async function(req, res, next){ + try{ + await permission.byGroup(req.user, ['app_sso_admin'], [req.params.group]); + + const parent = await Group.get(req.params.group); + const child = await Group.get(req.params.child); + + if(parent.dn === child.dn){ + return res.status(400).json({message: 'A group cannot contain itself.'}); + } + // Refuse rather than rely on the resolver's depth cap: a cycle makes + // "who is in this group" unanswerable, and the cap would quietly return + // a truncated answer instead of an error anyone would notice. + if(await Group.wouldCycle(req.params.group, child.dn)){ + return res.status(409).json({ + message: `"${req.params.child}" already contains "${req.params.group}" — nesting them would create a loop.` + }); + } + + const results = await parent.addMember({dn: child.dn}); + User.clearCache(); + return res.json({ + results, + message: `Nested ${req.params.child} inside ${req.params.group}.` + }); + }catch(error){ + if(error.name === 'TypeOrValueExistsError' || error.code === 20){ + return res.status(409).json({message: `"${req.params.child}" is already nested in "${req.params.group}".`}); + } + next(error); + } +}); + +// DELETE /api/group/:group/nested/:child — un-nest. +router.delete('/:group/nested/:child', async function(req, res, next){ + try{ + await permission.byGroup(req.user, ['app_sso_admin'], [req.params.group]); + + const parent = await Group.get(req.params.group); + const child = await Group.get(req.params.child); + const results = await parent.removeMember({dn: child.dn}); + User.clearCache(); + return res.json({ + results, + message: `Removed ${req.params.child} from ${req.params.group}.` + }); + }catch(error){ + // groupOfNames requires at least one member, so emptying a group is a + // schema violation rather than a permission problem. Surfacing the raw + // error as a 500 makes it look like a bug in the server; it is really a + // "you cannot do that, and here is why" -- the same reason the last user + // cannot be removed from a group either. + if(error.name === 'ObjectClassViolationError' || error.code === 65){ + return res.status(409).json({ + message: `"${req.params.child}" is the only member of "${req.params.group}". A group must keep at least one member — add another first.` + }); + } + next(error); + } +}); + router.put('/owner/:group/:uid', async function(req, res, next){ try{ @@ -92,6 +173,15 @@ router.put('/:group/:uid', async function(req, res, next){ message: `Added user ${req.params.uid} to ${req.params.group} group.` }); }catch(error){ + // Already a member -- surfaced as a plain 500 before, which read as a + // server fault for what is really a no-op. Common in practice because + // groupOfNames needs at least one member, so whoever creates a group is + // seeded into it and is then "added" again by the obvious next click. + if(error.name === 'TypeOrValueExistsError' || error.code === 20){ + return res.status(409).json({ + message: `"${req.params.uid}" is already a member of "${req.params.group}".` + }); + } next(error); } }); diff --git a/nodejs/routes/index.js b/nodejs/routes/index.js index 6e4c094..dd6ffc6 100755 --- a/nodejs/routes/index.js +++ b/nodejs/routes/index.js @@ -17,6 +17,13 @@ const values ={ titleIcon: conf.environment !== 'production' ? `` : '', name: conf.name, logo: conf.logo, + // Connection conventions the catalog needs to render "how to reach this" + // (conf/base.js `directory`). Safe to expose: a jump-host name and a default + // port are public connection info, not credentials. + directoryConf: { + jumpHost: (conf.directory && conf.directory.jumpHost) || '', + defaultSshPort: (conf.directory && conf.directory.defaultSshPort) || 22, + }, ...buildInfo, } diff --git a/nodejs/routes/user.js b/nodejs/routes/user.js index 0227a56..62da372 100755 --- a/nodejs/routes/user.js +++ b/nodejs/routes/user.js @@ -4,6 +4,7 @@ const router = require('express').Router(); const {User} = require('../models/user'); const {Group} = require('../models/group_ldap'); const permission = require('../utils/permission'); +const {groupCns} = require('../utils/user_groups'); const {UserVerification} = require('../models/verification'); const {InviteToken} = require('../models/token'); @@ -78,11 +79,17 @@ router.get('/me', async function(req, res, next){ // The shared client framework gates the UI on a single effective-rights // flag (the OIDC-client apps send the same key). Here "admin" means - // membership in app_sso_admin or the cross-app app_super_admin group; - // group-level gating still reads memberOf. - const groups = (user.memberOf || []).map(function(dn){ - return String(dn).split(',')[0].replace(/^cn=/i, ''); - }); + // membership in app_sso_admin or the cross-app app_super_admin group. + // + // Resolved via groupCns rather than read off `memberOf` directly: with + // nested groups, memberOf is only transitive when the directory carries + // the nestgroup overlay. Against a server without it, an admin who holds + // the group through nesting would get isAdmin=false here and silently + // lose the whole admin UI -- while still passing every server-side + // permission check, which resolves nesting properly. groupCns gives the + // same answer in both modes. + const groups = await groupCns(user); + user.groups = groups; user.isAdmin = groups.includes('app_sso_admin') || groups.includes(permission.SUPER_ADMIN_GROUP); return res.json(user); diff --git a/nodejs/tests/access_request.test.js b/nodejs/tests/access_request.test.js new file mode 100644 index 0000000..ba5e7cc --- /dev/null +++ b/nodejs/tests/access_request.test.js @@ -0,0 +1,235 @@ +'use strict'; + +// Self-service access requests, end to end: request -> approve -> the grant is +// real (visible through /api/discovery/me), plus the guards that keep the flow +// from being abused or double-applied. +// +// The seed `test` user is in app_sso_admin, so it is both the requester and an +// eligible approver here. That is unusual in production but exactly what makes +// a single-user test able to walk the whole loop. + +const { login, request, app } = require('./setup'); + +let token; +let siteSlug; +let hostSlug; +let hostId; +let accessGroupCn; + +// Unique per run: these create real LDAP groups and SQL rows, and a rerun must +// not collide with the previous run's leftovers. +const stamp = Date.now().toString(36); + +beforeAll(async () => { + token = await login(); + + siteSlug = `artest-site-${stamp}`; + const site = await request(app) + .post('/api/directory-admin/resources') + .set('auth-token', token) + .send({ name: `AR Test Site ${stamp}`, slug: siteSlug, kind: 'site' }); + expect(site.status).toBe(200); + + hostSlug = `artest-host-${stamp}`; + const host = await request(app) + .post('/api/directory-admin/resources') + .set('auth-token', token) + .send({ + name: `AR Test Host ${stamp}`, + slug: hostSlug, + kind: 'host', + parentSlug: siteSlug, + metadata: { ip: '10.99.99.9' }, + }); + expect(host.status).toBe(200); + hostId = host.body.results.id; + + // Creating a host auto-provisions __access / _admin. + accessGroupCn = `${siteSlug}_${hostSlug}_access`; + const adminGroupCn = `${siteSlug}_${hostSlug}_admin`; + + // The creator is seeded into both groups -- groupOfNames requires at least + // one member, so Group.add puts the owner's DN there -- and _admin is nested + // into _access, so membership of either grants access. A user who already + // has access cannot request it (correctly), so step out of both to be a + // legitimate requester. Removing only _access would leave the grant intact + // through the nesting, which is exactly the kind of thing these tests exist + // to catch. + for (const cn of [adminGroupCn, accessGroupCn]) { + await request(app) + .delete(`/api/group/${encodeURIComponent(cn)}/test`) + .set('auth-token', token); + } +}); + +describe('Access requests — the request half', () => { + let requestId; + + test('POST /api/access-requests creates a pending request on the member group', async () => { + const res = await request(app) + .post('/api/access-requests') + .set('auth-token', token) + .send({ slug: hostSlug, note: 'need it for testing' }); + + expect(res.status).toBe(200); + expect(res.body.results).toBeDefined(); + expect(res.body.results.status).toBe('pending'); + expect(res.body.results.uid).toBe('test'); + // Must target the _access group, never the _admin one: asking to use a + // resource may not silently escalate to administering it. + expect(res.body.results.groupCn).toBe(accessGroupCn); + requestId = res.body.results.id; + }); + + test('a second request for the same resource is rejected', async () => { + const res = await request(app) + .post('/api/access-requests') + .set('auth-token', token) + .send({ slug: hostSlug }); + expect(res.status).toBe(409); + }); + + test('GET /api/access-requests/mine lists it with the resource attached', async () => { + const res = await request(app).get('/api/access-requests/mine').set('auth-token', token); + expect(res.status).toBe(200); + const found = res.body.results.find(r => r.id === requestId); + expect(found).toBeDefined(); + expect(found.resource.slug).toBe(hostSlug); + }); + + test('GET /api/access-requests shows it to an approver', async () => { + const res = await request(app).get('/api/access-requests').set('auth-token', token); + expect(res.status).toBe(200); + expect(res.body.results.some(r => r.id === requestId)).toBe(true); + }); + + test('requesting an unknown resource is a 404', async () => { + const res = await request(app) + .post('/api/access-requests') + .set('auth-token', token) + .send({ slug: `no-such-resource-${stamp}` }); + expect(res.status).toBe(404); + }); +}); + +describe('Access requests — approval actually grants', () => { + let requestId; + + beforeAll(async () => { + const mine = await request(app).get('/api/access-requests/mine').set('auth-token', token); + const pending = mine.body.results.find(r => r.groupCn === accessGroupCn && r.status === 'pending'); + requestId = pending && pending.id; + expect(requestId).toBeDefined(); + }); + + test('the resource is NOT in /api/discovery/me before approval', async () => { + const res = await request(app).get('/api/discovery/me').set('auth-token', token); + expect(res.status).toBe(200); + expect(res.body.results.some(r => r.id === hostId)).toBe(false); + }); + + test('POST /:id/approve marks it approved', async () => { + const res = await request(app) + .post(`/api/access-requests/${requestId}/approve`) + .set('auth-token', token) + .send({ decisionNote: 'ok' }); + expect(res.status).toBe(200); + expect(res.body.results.status).toBe('approved'); + expect(res.body.results.decidedBy).toBe('test'); + }); + + test('approving twice is rejected', async () => { + const res = await request(app) + .post(`/api/access-requests/${requestId}/approve`) + .set('auth-token', token) + .send({}); + expect(res.status).toBe(409); + }); + + // The payoff, and the regression guard for the user.groups bug: /me resolved + // groups off req.user.groups, which does not exist on a User (it carries + // memberOf), so this endpoint used to return only isPublic resources no + // matter what the caller was actually a member of. + test('the resource IS in /api/discovery/me after approval', async () => { + const res = await request(app).get('/api/discovery/me').set('auth-token', token); + expect(res.status).toBe(200); + const found = res.body.results.find(r => r.id === hostId); + expect(found).toBeDefined(); + // And it answers "how do I reach it" rather than just naming the thing. + expect(found.resolvedAddress).toBe('10.99.99.9'); + }); + + test('an already-granted resource cannot be requested again', async () => { + const res = await request(app) + .post('/api/access-requests') + .set('auth-token', token) + .send({ slug: hostSlug }); + expect(res.status).toBe(409); + }); +}); + +describe('Admin access visibility', () => { + test('GET /api/directory-admin/access-summary counts the host\'s groups + members', async () => { + const res = await request(app) + .get('/api/directory-admin/access-summary') + .set('auth-token', token); + expect(res.status).toBe(200); + const summary = res.body.results[hostId]; + expect(summary).toBeDefined(); + // _access and _admin were both auto-created and linked. + expect(summary.groups.length).toBe(2); + expect(summary.groups.every(g => g.exists)).toBe(true); + // The approval above put `test` in the access group. + expect(summary.memberCount).toBeGreaterThanOrEqual(1); + }); + + test('GET /api/directory-admin/user-access/:uid answers the reverse question', async () => { + const res = await request(app) + .get('/api/directory-admin/user-access/test') + .set('auth-token', token); + expect(res.status).toBe(200); + expect(res.body.results.uid).toBe('test'); + const entry = res.body.results.resources.find(r => r.id === hostId); + expect(entry).toBeDefined(); + expect(entry.groupCn).toBe(accessGroupCn); + }); + + test('user-access for an unknown uid is a 404', async () => { + const res = await request(app) + .get('/api/directory-admin/user-access/definitely-not-a-user') + .set('auth-token', token); + expect(res.status).toBe(404); + }); +}); + +describe('Access requests — withdrawal', () => { + test('a requester can withdraw their own pending request', async () => { + // A second resource, so this does not disturb the approved one above. + const slug = `artest-host2-${stamp}`; + const host = await request(app) + .post('/api/directory-admin/resources') + .set('auth-token', token) + .send({ name: `AR Test Host2 ${stamp}`, slug, kind: 'host', parentSlug: siteSlug }); + expect(host.status).toBe(200); + + // Same as the top-level setup: step out of the auto-created groups the + // creator is seeded into, or this is a request for access already held. + for (const cn of [`${siteSlug}_${slug}_admin`, `${siteSlug}_${slug}_access`]) { + await request(app) + .delete(`/api/group/${encodeURIComponent(cn)}/test`) + .set('auth-token', token); + } + + const created = await request(app) + .post('/api/access-requests') + .set('auth-token', token) + .send({ slug }); + expect(created.status).toBe(200); + + const res = await request(app) + .delete(`/api/access-requests/${created.body.results.id}`) + .set('auth-token', token); + expect(res.status).toBe(200); + expect(res.body.results.status).toBe('cancelled'); + }); +}); diff --git a/nodejs/tests/nested_group.test.js b/nodejs/tests/nested_group.test.js new file mode 100644 index 0000000..a000b1c --- /dev/null +++ b/nodejs/tests/nested_group.test.js @@ -0,0 +1,136 @@ +'use strict'; + +// Nested groups: the API for putting a group inside a group, the cycle guard, +// and the thing that makes it worth doing -- membership resolving transitively +// through the chain. +// +// Fixture note that is easy to get wrong: groupOfNames requires at least one +// member, so whoever creates a group is seeded into it. `test` creates all +// three groups here and would therefore be a *direct* member of each, which +// would make "resolved via nesting" indistinguishable from "was already in it". +// Setup below strips that back so test's only direct membership is the +// innermost group -- and the strip has to happen after nesting, or removing the +// sole member would violate the objectClass. + +const { login, request, app } = require('./setup'); + +let token; +const stamp = Date.now().toString(36); +const A = `nesttest-a-${stamp}`; // outermost +const B = `nesttest-b-${stamp}`; // middle +const C = `nesttest-c-${stamp}`; // innermost, holds the user +// A second group nested into A purely so that un-nesting B later does not +// empty A -- groupOfNames requires at least one member, and the API correctly +// refuses (409) rather than leaving an invalid entry behind. +const D = `nesttest-d-${stamp}`; + +async function nest(parent, child) { + return request(app).put(`/api/group/${parent}/nested/${child}`).set('auth-token', token).send({}); +} + +beforeAll(async () => { + token = await login(); + + for (const cn of [A, B, C, D]) { + const res = await request(app) + .post('/api/group') + .set('auth-token', token) + .send({ name: cn, description: `nesting test ${cn}` }); + expect([200, 201]).toContain(res.status); + } + + expect((await nest(A, B)).status).toBe(200); + expect((await nest(B, C)).status).toBe(200); + expect((await nest(A, D)).status).toBe(200); + + // Now that A holds B and B holds C, neither would be left memberless. + for (const cn of [A, B]) { + const res = await request(app).delete(`/api/group/${cn}/test`).set('auth-token', token); + expect(res.status).toBe(200); + } +}); + +describe('Nested groups — API guards', () => { + test('nesting the same pair twice is a 409, not a duplicate', async () => { + const res = await nest(A, B); + expect(res.status).toBe(409); + }); + + test('a group cannot contain itself', async () => { + const res = await nest(A, A); + expect(res.status).toBe(400); + }); + + // The guard that matters: without it the resolver would silently return a + // depth-capped answer instead of an error anyone would notice. + test('a direct cycle is refused (A contains B, so B may not contain A)', async () => { + const res = await nest(B, A); + expect(res.status).toBe(409); + expect(res.body.message).toMatch(/loop/i); + }); + + test('an indirect cycle is refused too (A>B>C, so C may not contain A)', async () => { + const res = await nest(C, A); + expect(res.status).toBe(409); + }); +}); + +describe('Nested groups — resolution', () => { + test('membership resolves through the whole chain', async () => { + const res = await request(app).get('/api/group?member=test').set('auth-token', token); + expect(res.status).toBe(200); + expect(res.body.results).toContain(C); // direct + expect(res.body.results).toContain(B); // via C + expect(res.body.results).toContain(A); // via B -> C + }); + + test('GET /:group/effective separates direct members from nested ones', async () => { + const res = await request(app).get(`/api/group/${A}/effective`).set('auth-token', token); + expect(res.status).toBe(200); + const { direct, nestedGroups, effective } = res.body.results; + + expect(nestedGroups.map(g => g.cn)).toContain(B); + // `direct` is users only -- a nested group must never be reported as one. + expect(direct.every(dn => !/,ou=groups,/i.test(dn))).toBe(true); + // test is not listed on A at all, yet is effectively a member two levels down. + expect(direct.some(dn => /cn=test,/i.test(dn))).toBe(false); + expect(effective.some(dn => /cn=test,/i.test(dn))).toBe(true); + }); +}); + +describe('Nested groups — un-nesting', () => { + test('DELETE removes the nesting and the membership it carried', async () => { + // Before: A holds B (which holds C, which holds test) and D. + const before = await request(app).get(`/api/group/${A}/effective`).set('auth-token', token); + expect(before.body.results.nestedGroups.map(g => g.cn)).toContain(B); + expect(before.body.results.effective.some(dn => /cn=test,/i.test(dn))).toBe(true); + + const res = await request(app) + .delete(`/api/group/${A}/nested/${B}`) + .set('auth-token', token); + expect(res.status).toBe(200); + + const after = await request(app).get(`/api/group/${A}/effective`).set('auth-token', token); + expect(after.body.results.nestedGroups.map(g => g.cn)).not.toContain(B); + expect(after.body.results.nestedGroups.map(g => g.cn)).toContain(D); // untouched + + // test still resolves to B and C directly/through C; only the A path via + // B is gone. It is deliberately NOT asserted that test loses A entirely: + // D is also nested in A and test created D, so that path remains -- which + // is itself a fair illustration of why "who can reach this" has to be + // computed rather than eyeballed. + const groups = await request(app).get('/api/group?member=test').set('auth-token', token); + expect(groups.body.results).toContain(C); + expect(groups.body.results).toContain(B); + }); + + test('un-nesting the last member is refused rather than emptying the group', async () => { + // B now holds only C. Removing it would leave B with no members at all, + // which groupOfNames forbids. + const res = await request(app) + .delete(`/api/group/${B}/nested/${C}`) + .set('auth-token', token); + expect(res.status).toBe(409); + expect(res.body.message).toMatch(/at least one member/i); + }); +}); diff --git a/nodejs/utils/permission.js b/nodejs/utils/permission.js index a2fa2ca..15fc2e0 100644 --- a/nodejs/utils/permission.js +++ b/nodejs/utils/permission.js @@ -5,22 +5,27 @@ const {Group} = require('../models/group_ldap'); const SUPER_ADMIN_GROUP = 'app_super_admin'; let byGroup = async function(user, groups, ownerOf){ + // Membership is resolved once, transitively: a user placed in an admin group + // through a nested group is as much a member as one listed on it directly. + // Checking `group.member.includes(user.dn)` per group -- as this used to -- + // only ever sees the literal member list and would deny them. + let memberOfCns = []; try{ - let superAdmin = await Group.get(SUPER_ADMIN_GROUP); - if(superAdmin.member.includes(user.dn)) return true + memberOfCns = await Group.list(user.dn); }catch(error){ - // group not found, continue checking + // Fall through to the per-group checks below rather than hard-failing; + // they still catch direct membership if the resolver is unavailable. } + if(memberOfCns.includes(SUPER_ADMIN_GROUP)) return true; + for(let group of groups){ - try{ - group = await Group.get(group); - if(group.member.includes(user.dn)) return true - }catch(error){ - // group not found, continue checking - } + if(memberOfCns.includes(group)) return true; } + // `owner` is deliberately NOT transitive. It designates accountable people, + // and inheriting ownership through a nested group would hand approval rights + // to anyone transitively in it -- an escalation nobody asked for. for(let group of ownerOf || []){ try{ group = await Group.get(group); diff --git a/nodejs/utils/ui.js b/nodejs/utils/ui.js index 545e6ef..d015ef5 100644 --- a/nodejs/utils/ui.js +++ b/nodejs/utils/ui.js @@ -38,6 +38,10 @@ module.exports = { // app-base.js, which reveals .group-required- for each group the user is // in (plus the synthetic `admin` group when user/me reports isAdmin). nav: [ + // Ungated on purpose: the catalog is the one page that exists for + // ordinary users. Before this, every nav item was admin-only and a + // non-admin had no signposted destination at all. + {href: '/', icon: 'fa-solid fa-compass', label: 'Catalog', groups: []}, {href: '/users', icon: 'fa-solid fa-users', label: 'Users', groups: ['app_sso_admin', 'admin']}, {href: '/groups', icon: 'fa-solid fa-users-viewfinder', label: 'Groups', groups: ['app_sso_admin', 'admin']}, {href: '/directory', icon: 'fa-solid fa-server', label: 'Directory', groups: ['app_sso_admin', 'app_sso_directory_admin', 'admin']}, diff --git a/nodejs/utils/user_groups.js b/nodejs/utils/user_groups.js new file mode 100644 index 0000000..7bb6d9a --- /dev/null +++ b/nodejs/utils/user_groups.js @@ -0,0 +1,56 @@ +'use strict'; + +// Resolve a request user's LDAP group CNs. +// +// Why this exists: `req.user` is a `User.get()` result, which carries +// `memberOf` -- a list of full group DNs -- and has no `groups` property at +// all. Anything reading `req.user.groups` therefore silently sees an empty +// list rather than failing, which is how GET /api/discovery/me came to return +// only `isPublic` resources for every human caller, and how +// isDirectoryAdmin() came to be false even for real directory admins. +// +// routes/user.js:83 already derives the admin gate from `memberOf` the same +// way, so the overlay is known to be populated in production; the Group.list() +// fallback covers a user object assembled without it (and costs an LDAP round +// trip, so it is genuinely the fallback). + +const { Group } = require('../models/group_ldap'); + +// 'cn=app_sso_admin,ou=groups,dc=example,dc=com' -> 'app_sso_admin' +function cnFromDn(dn) { + return String(dn).split(',')[0].replace(/^cn=/i, ''); +} + +async function groupCns(user) { + if (!user || user.isMachine) return []; + + // Group.list(dn) resolves nested groups transitively. `memberOf` cannot: the + // memberof overlay records only direct membership, so a user who reaches a + // resource group through a nested group is absent from it entirely. That + // makes memberOf a fallback for when there is no DN to query with, never the + // preferred source -- reading it first would silently drop every nested grant. + if (user.dn) { + try { + return await Group.list(user.dn); + } catch (err) { + console.error(`groupCns: LDAP lookup failed for ${user.uid}:`, err.message); + } + } + + if (Array.isArray(user.memberOf)) return user.memberOf.map(cnFromDn); + // memberOf is single-valued when the user is in exactly one group. + if (user.memberOf) return [cnFromDn(user.memberOf)]; + + return []; +} + +// The shape @simpleworkjs/directory-schema's isDirectoryAdmin() expects: it +// matches against `.groups`, which the raw request user does not have. +async function withGroups(user) { + if (!user) return user; + return Object.assign(Object.create(Object.getPrototypeOf(user) || Object.prototype), user, { + groups: await groupCns(user), + }); +} + +module.exports = { groupCns, withGroups, cnFromDn }; diff --git a/nodejs/views/directory.ejs b/nodejs/views/directory.ejs index f95eeab..4d109a2 100644 --- a/nodejs/views/directory.ejs +++ b/nodejs/views/directory.ejs @@ -15,6 +15,13 @@ +
+ + + + +
@@ -31,6 +38,7 @@ Resource IP / Address + Access Actions @@ -52,6 +60,7 @@ {{#metadata.ip}}
IP: {{metadata.ip}}
{{/metadata.ip}} {{#metadata.address}}
URL: {{metadata.address}}
{{/metadata.address}} + {{{accessHtml}}} + + {{ /nested }} + {{ ^hasNested }} +
  • No groups nested here.
    • + {{ /hasNested }} + +
    + +
    + {{ #toNest }} + {{ cn }} + {{ /toNest }} +
    +
    + +

      diff --git a/nodejs/views/landing.ejs b/nodejs/views/landing.ejs index 0a97242..6bf8dcb 100644 --- a/nodejs/views/landing.ejs +++ b/nodejs/views/landing.ejs @@ -1,136 +1,329 @@ <%- include('top') %>
      -

      SSO Portal

      -

      Explore and access all your services in one place.

      - My Profile +

      <%- name %> Portal

      +

      Everything the lab offers — what you can reach, and how to reach it.

      + My Profile
      -

      My Apps & Services

      -