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

      -