On Fri, May 25, 2001 at 07:46:24AM -0400, Jan Harkes wrote:
> > Also, I'm trying to change the user id for the administrative user in 
> > pdbtool, but not all references to that id are changed.  I want to free 
> > up that id so I can match my current system user ids for consistency 
> > across the system.  If I change the id and do a 'list', the 
> > System:Administrators owner id still shows the old value and now I can't 
> > do anything as the coda admin until I change it back.  Same with any 
> > groups that are owned by the user whose id I'm trying to change.  Is 
> > this a bug in pdbtool?
> 
> I guess the pdbtools isn't exhaustive enough in hunting down all id's
> that need to be renumbered. It will work when the changing userid is
> only a member of a group, so yes, this is a bug. I'll look into it.

I found several bugs in the implementation of 'pdbtool ci' that result
in inconsistencies in the pdb databases.

- Group owner id wasn't updated.
- Traversal of the groups_or_members array was broken. As a result users
  who's uid changed and that were a member of a group, are not listed as
  group members in the group information.
- Member of information was updated in such a way that a group would
  be listed as a member of a user (shouldn't be possible).

I just committed a version into CVS that fixes these bugs and also has
added the functionality to 'pdbtool cm' to fix inconsistencies that
occur as a result of these bugs (same patch follows here).

Jan

Index: pdb.c
===================================================================
RCS file: /afs/cs/project/coda-src/cvs/coda/coda-src/al/pdb.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -u -r1.9 -r1.10
--- pdb.c	1999/09/14 19:00:40	1.9
+++ pdb.c	2001/05/25 16:29:57	1.10
@@ -425,6 +425,99 @@
 }
 
 
+void PDB_bugfixes(void)
+{
+    PDB_HANDLE h;
+    /* fixups for old bugs */
+    int32_t id, id2;
+    pdb_array_off off, off2;
+    int rc;
+    PDB_profile p, r;
+
+    h = PDB_db_open(O_RDWR);
+
+    while ( (rc = PDB_db_nextkey(h, &id)) ) {
+	if ( rc == -1 ) continue; 
+	PDB_readProfile(h, id, &p);
+
+	if (PDB_ISGROUP(p.id)) {
+	    /* BUG: forgot to update owner_id when changing a user's uid */
+	    PDB_lookupByName(p.owner_name, &id);
+	    if (p.owner_id != id) {
+		fprintf(stderr, "Group %d owner name %s didn't match owner id %d, FIXED\n", p.id, p.owner_name, p.owner_id);
+		p.owner_id = id;
+		PDB_writeProfile(h, &p);
+	    }
+
+	    /* BUG: we added userid's to a group's member_of list */
+again:
+	    id = pdb_array_head(&p.member_of, &off);
+	    while(id != 0) {
+		if (PDB_ISUSER(id)) {
+		    fprintf(stderr, "Group %d was listed as a member of userid %d, FIXED\n", p.id, id);
+		    pdb_array_del(&p.member_of, id);
+		    PDB_updateCps(h, &p);
+		    PDB_writeProfile(h, &p);
+		    goto again;
+		}
+		id = pdb_array_next(&p.member_of, &off);
+	    }
+
+	    /* BUG: we forgot to change userid's in a group's groups_or_members
+	     *      list (fix part 1, removes non-existing or non-member
+	     *      userids) */
+again2:
+	    id = pdb_array_head(&p.groups_or_members, &off);
+	    while(id != 0) {
+		if (PDB_ISUSER(id)) {
+		    PDB_readProfile(h, id, &r);
+		    id2 = pdb_array_head(&r.member_of, &off2);
+		    while (id2 != 0) {
+			if (id2 == p.id) break;
+			id2 = pdb_array_next(&r.member_of, &off2);
+		    }
+		    if (id2 == 0) {
+			pdb_array_del(&p.groups_or_members, id);
+			PDB_updateCps(h, &p);
+			PDB_writeProfile(h, &p);
+			fprintf(stderr, "Group %d had nonexisting member %d, FIXED\n", p.id, id);
+			PDB_freeProfile(&r);
+			goto again2;
+		    }
+		    PDB_freeProfile(&r);
+		}
+		id = pdb_array_next(&p.groups_or_members, &off);
+	    }
+	}
+	else /* PDB_ISUSER(p.id) */
+	{
+	    /* BUG: we forgot to change userid's in a group's groups_or_members
+	     *      list (fix part 2, adds missing members to groups)*/
+	    id = pdb_array_head(&p.member_of, &off);
+	    while (id != 0) {
+		if (PDB_ISGROUP(id)) {
+		    PDB_readProfile(h, id, &r);
+		    id2 = pdb_array_head(&r.groups_or_members, &off2);
+		    while (id2 != 0) {
+			if (id2 == p.id) break;
+			id2 = pdb_array_next(&r.groups_or_members, &off2);
+		    }
+		    if (id2 == 0) {
+			fprintf(stderr, "Group %d was missing member %d, FIXED\n", id, p.id);
+			pdb_array_add(&r.groups_or_members, p.id);
+			PDB_updateCps(h, &r);
+			PDB_writeProfile(h, &r);
+		    }
+		    PDB_freeProfile(&r);
+		}
+		id = pdb_array_next(&p.member_of, &off);
+	    }
+	}
+	PDB_freeProfile(&p);
+    }
+    PDB_db_close(h);
+}
+
 void PDB_changeId(int32_t oldId, int32_t newId)
 {
 	PDB_HANDLE h;
@@ -466,35 +559,44 @@
 	else
 		PDB_db_update_maxids(h, newId, 0, PDB_MAXID_SET);
 
-	/* update groups is member of */
+	/* update groups we are a member of */
 	nextid = pdb_array_head(&(r.member_of), &off);
 	while(nextid != 0){
 		PDB_readProfile(h, nextid, &p);
 		CODA_ASSERT(p.id != 0);
 		pdb_array_del(&(p.groups_or_members), oldId);
 		pdb_array_add(&(p.groups_or_members), newId);
+
 		/* Don't need CPS updates */
 		PDB_writeProfile(h, &p);
 		PDB_freeProfile(&p);
-		nextid = pdb_array_next(&(r.groups_or_members), &off);
+		nextid = pdb_array_next(&(r.member_of), &off);
 	}
 
-	/* update members */
+	/* update members or ownership */
 	nextid = pdb_array_head(&(r.groups_or_members), &off);
-	while(nextid != 0){
-		PDB_readProfile(h, nextid, &p);
-		CODA_ASSERT(p.id != 0);
+	while(nextid != 0) {
+	    PDB_readProfile(h, nextid, &p);
+	    CODA_ASSERT(p.id != 0);
+
+	    if (PDB_ISGROUP(oldId)) {
 		pdb_array_del(&(p.member_of), oldId);
 		pdb_array_add(&(p.member_of), newId);
-		if(PDB_ISGROUP(oldId)){
-			if(PDB_ISGROUP(p.id))
-				PDB_updateCps(h, &p);
-			else
-				PDB_updateCpsSelf(h, &p);
+	    } else {
+		if (PDB_ISGROUP(p.id)) {
+		    if(p.owner_id == oldId)
+			p.owner_id = newId;
 		}
-		PDB_writeProfile(h, &p);
-		PDB_freeProfile(&p);
-		nextid = pdb_array_next(&(r.groups_or_members), &off);
+	    }
+
+	    if(PDB_ISGROUP(p.id))
+		PDB_updateCps(h, &p);
+	    else
+		PDB_updateCpsSelf(h, &p);
+
+	    PDB_writeProfile(h, &p);
+	    PDB_freeProfile(&p);
+	    nextid = pdb_array_next(&(r.groups_or_members), &off);
 	}
 
 	PDB_db_close(h);
Index: pdb.h
===================================================================
RCS file: /afs/cs/project/coda-src/cvs/coda/coda-src/al/pdb.h,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -u -r1.2 -r1.3
--- pdb.h	1999/05/07 04:04:19	1.2
+++ pdb.h	2001/05/25 16:29:57	1.3
@@ -70,6 +70,9 @@
 int PDB_nameInUse(char *name);
 void PDB_changeId(int32_t oldid, int32_t newid);
 
+/* fix known problems in PDB profiles created by older versions of pdbtool */
+void PDB_bugfixes(void);
+
 /* internal packing functions */
 void pdb_pack(PDB_profile *r, void **data, size_t *size);
 void pdb_unpack(PDB_profile *r, void *data, size_t size);
Index: pdbtool.c
===================================================================
RCS file: /afs/cs/project/coda-src/cvs/coda/coda-src/al/pdbtool.c,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -u -r1.13 -r1.14
--- pdbtool.c	2001/01/19 12:58:02	1.13
+++ pdbtool.c	2001/05/25 16:29:57	1.14
@@ -362,12 +362,17 @@
 }
 
 /* COMPACT DATABASES */
-void tool_compact(int argc,char *argv[]){
+void tool_compact(int argc,char *argv[])
+{
 	PDB_HANDLE h;
+
 	if(check_args_num(argc,1)){
 		printf("Usage: cm\n");
 		return;
 	}
+	/* fix database consistency bugs */
+	PDB_bugfixes();
+
 	h = PDB_db_open(O_RDWR);
 	PDB_db_compact(h);
 	PDB_db_close(h);
@@ -757,7 +762,7 @@
 	printf("cu <newusername> <userid>\tclone a user\n");
 	printf("ag <groupid/name> <id/name>\tadd a group or user to a group\n");
 	printf("rg <groupid/name> <id/name>\tremove a group or user from a group\n");
-	printf("d <id/name>\t\t\t\tdelete a user or a group\n");
+	printf("d <id/name>\t\t\tdelete a user or a group\n");
 	printf("cm\t\t\t\tcompact the database (RARE)\n");
 	printf("ci <name> <newid>\t\tchange the Id of a user or group\n");
 	printf("cn <id> <newname>\t\tchange the Name of a user or group\n");