Merge pull request #77 from theta42/fix-account-editing-bugs

Fix account-editing bugs; add editable group membership from profile

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

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
+	// manager (COSINE, SUP distinguishedName) and memberOf (from the memberof
-	// a bare string for a single value and an array for multiple -- normalize
+	// overlay) are both multi-valued; ldapts returns a bare string for a
-	// to always be an array of DNs.
+	// single value and an array for multiple -- normalize both to always be
-	data.manager = [].concat(data.manager || []).filter(Boolean);
+	// 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 });

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",

nodejs/package.json

@@ -1,6 +1,6 @@
 {
   "name": "t42-sso-manager",
-  "version": "1.1.7",
+  "version": "1.1.8",
   "private": true,
   "author": [
     {

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);
 			}

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 @@
 			<div class="profile-body" jq-repeat="user">
 				<div class="card-body profile-body-{{uid}}">
 					<h2><i>User Name:</i> <b>{{uid}}</b></h2>
-					<i>Name:</i> <b>{{givenName}} {{sn}}</b><br />
+					{{^isServiceAccount}}<i>Name:</i> <b>{{givenName}} {{sn}}</b><br />{{/isServiceAccount}}
 					<i>Email:</i> <b>{{mail}}</b>
 					{{#emailVerified}}<span class="badge bg-success ms-1"><i class="fa-solid fa-circle-check"></i> Verified</span>{{/emailVerified}}
 					<br />
@@ -247,7 +279,7 @@
 
 						<div class="mb-3">
 							<label class="form-label">Mobile Phone</label>
-							<input type="text" class="form-control" name="mobile" placeholder="9175551234" validate=":9" value="{{mobile}}" />
+							<input type="text" class="form-control" name="mobile" placeholder="9175551234" value="{{mobile}}" />
 						</div>
 						<div class="mb-3">
 							<label class="form-label">Home Directory</label>
@@ -282,7 +314,7 @@
 			</div>
 			<div class="card-header shadow actionMessage" style="display:none">
 			</div>
-			<div class="card-body" style="padding-bottom:0">
+			<div class="card-body">
 				<div class="table-responsive">
 					<table class="table">
 						<thead>
@@ -292,15 +324,28 @@
 							<th>
 								Description
 							</th>
+							<th class="group-required group-required-app_sso_admin"></th>
 						</thead>
 						<tbody jq-repeat="mygroups">
 							<tr>
 								<td>{{cn}}</td>
 								<td>{{description}}</td>
+								<td class="text-end group-required group-required-app_sso_admin">
+									<button type="button" class="btn btn-sm btn-outline-danger" title="Remove from group" onclick="removeFromGroup('{{cn}}', this)">
+										<i class="fa-solid fa-xmark"></i>
+									</button>
+								</td>
 							</tr>
 						</tbody>
 					</table>
 				</div>
+				<div class="group-required group-required-app_sso_admin">
+					<label class="form-label small">Add to group</label>
+					<div class="d-flex gap-2 align-items-start">
+						<div id="add-group-select" class="flex-grow-1"></div>
+						<button type="button" class="btn btn-outline-dark" onclick="addToGroups(this)">Add</button>
+					</div>
+				</div>
 			</div>
 		</div>
 	</div>