From 5d7c0bd59473be11f3c6ed74a545821c358dff88 Mon Sep 17 00:00:00 2001 From: William Mantly Date: Fri, 17 Jul 2026 11:00:50 -0400 Subject: [PATCH] Fix account-editing bugs from real-world feedback, add editable group membership - Edit form's Mobile Phone field was effectively required (stray validate attribute) -- removed. - Service account profiles always showed the literal filler name "Service Account" -- hidden now, since it's not meaningful. Required computing isServiceAccount in User.get(), not just listDetail(). - Fresh service accounts could look uncategorized (missing from the Service Accounts tab, wrong isServiceAccount) for up to 5 minutes after creation, due to a cache-staleness race in the create route -- the user gets cached via User.get() before the route marks it as a service account. Cleared and re-fetched after marking. - memberOf came back as a bare string instead of a one-element array for users in exactly one group, causing client-side permission checks to iterate character-by-character and incorrectly deny access -- normalized alongside the existing manager normalization. - Added editable group membership on the profile page ("My groups"), admin-only, using the existing per-group member endpoints. Bumps to v1.1.8. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KDEx8ghuZR61pqPXc6da9C --- CHANGELOG.md | 14 ++++++++++- nodejs/models/user_ldap.js | 18 +++++++++++--- nodejs/package-lock.json | 4 +-- nodejs/package.json | 2 +- nodejs/routes/user.js | 8 +++++- nodejs/views/profile.ejs | 51 +++++++++++++++++++++++++++++++++++--- 6 files changed, 85 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81d94ed..66faf4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,17 @@ correspond to git tags (`vX.Y.Z`) and `nodejs/package.json`'s `version`. ## [Unreleased] +## [1.1.8] - 2026-07-17 + +### Added +- Group membership is now editable directly from a user's profile page ("My groups" -- add via a group-name picker, remove with a button per row), instead of only from each group's own card on the Groups page. Admin-only, using the existing per-group member add/remove endpoints. + +### Fixed +- The Edit Profile form's Mobile Phone field had a stray `validate=":9"` making it effectively required (submission was blocked with "Please fix the form errors" if left blank) -- it was always meant to be optional, matching the "Add user" form. Removed. +- A service account's profile always showed `Name: Service Account` -- every service account has the same literal filler given/last name (a schema-satisfying placeholder, not meant to be shown), making them indistinguishable by name. The Name line is now hidden for service accounts. +- The Users page's Service Accounts tab, and a freshly-created service account's own profile, could appear empty/not-a-service-account for up to 5 minutes right after creation. Creating a user caches it via `User.get()` *before* the route handler marks it as a service account (group membership), so the cached copy had `isServiceAccount` stuck wrong until the cache TTL expired. Now cleared and re-fetched immediately after marking. +- A user belonging to exactly one LDAP group had their `memberOf` attribute returned as a bare string instead of a one-element array (ldapts's normal behavior for single-valued attributes) -- client-side permission checks (`for(let group of user.memberOf)`) would then iterate the DN character-by-character instead of once, causing pages gated on that group (e.g. Groups) to incorrectly show "You do not have permission to be here." Normalized `memberOf` to always be an array, same fix already applied to `manager`. + ## [1.1.7] - 2026-07-17 ### Changed @@ -61,7 +72,8 @@ First tagged release. Establishes the `vX.Y.Z` tag convention that the in-app up - Unix/POSIX and LDAP bind-only service account support, distinct from real-person accounts. - Merged OAuth Apps + LDAP Info into a single Integrations page. -[Unreleased]: https://github.com/theta42/sso-manager-node/compare/v1.1.7...HEAD +[Unreleased]: https://github.com/theta42/sso-manager-node/compare/v1.1.8...HEAD +[1.1.8]: https://github.com/theta42/sso-manager-node/compare/v1.1.7...v1.1.8 [1.1.7]: https://github.com/theta42/sso-manager-node/compare/v1.1.6...v1.1.7 [1.1.6]: https://github.com/theta42/sso-manager-node/compare/v1.1.5...v1.1.6 [1.1.5]: https://github.com/theta42/sso-manager-node/compare/v1.1.4...v1.1.5 diff --git a/nodejs/models/user_ldap.js b/nodejs/models/user_ldap.js index 331a61b..11f9e2a 100644 --- a/nodejs/models/user_ldap.js +++ b/nodejs/models/user_ldap.js @@ -210,10 +210,13 @@ const user_parse = function(data){ data.isActive = data.pwdAccountLockedTime ? '' : 'active'; data.isInactive = data.pwdAccountLockedTime ? 'inactive' : ''; - // manager (COSINE, SUP distinguishedName) is multi-valued; ldapts returns - // a bare string for a single value and an array for multiple -- normalize - // to always be an array of DNs. - data.manager = [].concat(data.manager || []).filter(Boolean); + // manager (COSINE, SUP distinguishedName) and memberOf (from the memberof + // overlay) are both multi-valued; ldapts returns a bare string for a + // single value and an array for multiple -- normalize both to always be + // an array, or app-base.js's `for(let group of user.memberOf)` silently + // iterates a single DN string character-by-character instead of once. + data.manager = [].concat(data.manager || []).filter(Boolean); + data.memberOf = [].concat(data.memberOf || []).filter(Boolean); return data; } @@ -348,6 +351,13 @@ User.get = async function(data, key) { const verif = await UserVerification.getOrCreate(obj.uid); + // Same membership check as User.listDetail() -- see the comment there. + try{ + const svcGroup = await Group.get('app_sso_service_account'); + const serviceAccountDNs = new Set((svcGroup.member || []).map(dn => dn.toLowerCase())); + obj.isServiceAccount = serviceAccountDNs.has(String(obj.dn).toLowerCase()) ? 'yes' : ''; + }catch(error){ obj.isServiceAccount = ''; } + // Auto-flag legacy MD5 password users — persist so subsequent cache hits see it if (isLegacyMD5 && !verif.password_must_change) { await verif.update({ password_must_change: true }); diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index a23de5b..f1ecf3d 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,12 +1,12 @@ { "name": "t42-sso-manager", - "version": "1.1.7", + "version": "1.1.8", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "t42-sso-manager", - "version": "1.1.7", + "version": "1.1.8", "license": "MIT", "dependencies": { "@fortawesome/fontawesome-free": "^7.3.0", diff --git a/nodejs/package.json b/nodejs/package.json index bb9ab67..b50446c 100755 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -1,6 +1,6 @@ { "name": "t42-sso-manager", - "version": "1.1.7", + "version": "1.1.8", "private": true, "author": [ { diff --git a/nodejs/routes/user.js b/nodejs/routes/user.js index 154836a..e327fd0 100755 --- a/nodejs/routes/user.js +++ b/nodejs/routes/user.js @@ -25,7 +25,7 @@ router.post('/', async function(req, res, next){ req.body.created_by = req.user.uid req.body.manager = [req.user.dn]; - const user = await User.add(req.body); + let user = await User.add(req.body); const verif = await UserVerification.getOrCreate(user.uid); const updates = { password_must_change: true }; if (req.body.tosAgree) updates.tos_accepted = true, updates.tos_accepted_at = Date.now(); @@ -38,6 +38,12 @@ router.post('/', async function(req, res, next){ try { const group = await Group.get('app_sso_service_account'); await group.addMember(user); + // User.add() already cached `user` (via its own internal + // User.get()) before this group membership existed, so the + // cached isServiceAccount would be stuck wrong for 5 minutes + // (the cache TTL) without this -- re-fetch after clearing. + User.clearCache(); + user = await User.get(user.uid); } catch (error) { console.error(`user.add: failed to mark ${user.uid} as a service account:`, error.message); } diff --git a/nodejs/views/profile.ejs b/nodejs/views/profile.ejs index 696cae0..d75cbeb 100644 --- a/nodejs/views/profile.ejs +++ b/nodejs/views/profile.ejs @@ -18,12 +18,40 @@ async function renderUserGroups(user){ try{ let res = await app.api.get('group/?detail=true&member='+user.uid); + $.scope.mygroups.empty(); $.scope.mygroups.push(...res.results); }catch(error){ console.error('renderUserGroups error:', error) } } + async function removeFromGroup(cn, btn){ + const $row = $(btn).closest('tr'); + const confirmed = await app.util.actionConfirm(`Remove ${currentUser.uid} from "${cn}"?`, $row, 'warning'); + if (!confirmed) return; + app.api.delete('group/' + encodeURIComponent(cn) + '/' + encodeURIComponent(currentUser.uid), function(error, data){ + if(error){ app.util.actionMessage((data && data.message) || 'Failed to remove from group', $row, 'danger'); return; } + $.scope.mygroups.remove('cn', cn); + }); + } + + var addGroupSelect; + async function addToGroups(btn){ + const cns = addGroupSelect.get(); + if(!cns.length) return; + const $card = $(btn).closest('.card-body'); + for(const cn of cns){ + await new Promise(function(resolve){ + app.api.put('group/' + encodeURIComponent(cn) + '/' + encodeURIComponent(currentUser.uid), {}, function(error, data){ + if(error) app.util.actionMessage((data && data.message) || `Failed to add to "${cn}"`, $card, 'danger'); + resolve(); + }); + }); + } + addGroupSelect.clear(); + renderUserGroups(currentUser); + } + async function determinUser(){ if(location.pathname.includes('/users/')){ let uid = location.pathname.replace('/users/', ''); @@ -95,6 +123,10 @@ renderProfile(currentUser); renderUserGroups(currentUser); + addGroupSelect = app.ui.groupSelect('#add-group-select', { + name: 'groups', values: [], placeholder: 'Type a group name…', + }); + // API Tokens are self-service only — never shown when an admin is // viewing someone else's profile via /users/:uid. if(isOwnProfile){ @@ -159,7 +191,7 @@

User Name: {{uid}}

- Name: {{givenName}} {{sn}}
+ {{^isServiceAccount}}Name: {{givenName}} {{sn}}
{{/isServiceAccount}} Email: {{mail}} {{#emailVerified}} Verified{{/emailVerified}}
@@ -247,7 +279,7 @@
- +
@@ -282,7 +314,7 @@
-
+
@@ -292,15 +324,28 @@ + +
Description
{{cn}} {{description}} + +
+
+ +
+
+ +
+