On Thu, Jan 18, 2001 at 11:11:13AM +0100, Steffen Neumann wrote:
> We have a local patch that'll make sure coda groups 
> have a negative id. We also fixed the parsing with fscanf,
> though it still has problems, e.g. if a Unix group has no members.
> 
> Is there any use for our patch ? 
> Or did we break something else ?

I've merged your patch and extended it a bit.

- Refusing to add userid or groupid 0, there is no "root" user in Coda.
- Skipping users with a negative userid.
- Detecting the creation of an already existing group.
- When a groupid is positive, force the group owner to "System".

There are some users and groups that need to be in the pdb database (in
passwd/group format).

coda.users
----------
Anonymous:*:776:0::/:
System:*:777:0::/:

coda.groups
-----------
System%AnyUser:*:-101:System
System%Administrators:*:-204:System


I can't find any references to "Anonymous", PRS_ANONYMOUSID or 776
anywhere in the code, so that one is probably obsolete.

The id's for System, System:Administrators and System:AnyUser are
hardcoded in only 2 or 3 places. I can replace these with pdb lookups.
System would then actually not be necessary anymore, although we still
need some default owner that we can use for groups.

It looks like System:AnyUser actually should be a user instead of a
group. It is logically a group where everyone is automatically a member
of, but the server code uses it mostly as an `Unauthenticated User'
identifier and adds any ACL rights given to the "System:AnyUser user" to
those of authenticated users that checks the ACL.

In any case, I was thinking of the following patch to pdbtool.

Jan

Index: pdbtool.c
===================================================================
RCS file: /afs/cs/project/coda-src/cvs/coda/coda-src/al/pdbtool.c,v
retrieving revision 1.11
diff -u -u -r1.11 pdbtool.c
--- pdbtool.c	2000/08/22 12:58:47	1.11
+++ pdbtool.c	2001/01/18 09:47:55
@@ -604,9 +604,18 @@
     /* recreate all users */
     userfile = fopen(argv[1], "r");
     while(1) {
-	rc = fscanf(userfile, "%[^:]:%*[^:]:%d:%*s\n", user, &user_id);
+	rc = fscanf(userfile, "%[^:]:%*[^:]:%d:%*[^\n]\n", user, &user_id);
 	if (rc < 0) break;
 
+	if (user_id == 0) {
+	    printf("Userid 0 must be avoided, skipping entry for %s\n", user);
+	    continue;
+	}
+	if (user_id < 0) {
+	    printf("Skipping user %s with a negative id %d\n", user, user_id);
+	    continue;
+	}
+
 	/* create user */
 	PDB_lookupById(user_id, &s);
 	if (s) {
@@ -625,15 +634,28 @@
     /* recreate groups */
     groupfile = fopen(argv[2], "r");
     while (1) {
-	rc = fscanf(groupfile, "%[^:]:%*[^:]:%d:%s\n",
+	rc = fscanf(groupfile, "%[^:]:%*[^:]:%d:%[^\n]\n",
 		    group, &group_id, owner_and_members);
 	if (rc < 0) break;
 
+	if (group_id == 0) {
+	    printf("Groupid 0 must be avoided, skipping entry for %s\n",
+		   group);
+	    continue;
+	}
+
 	/* restore the :'s in the group name */
 	s = group; while ((s = strchr(s, '%')) != NULL) *s = ':';
 
 	owner = strtok(owner_and_members, ",");
 
+	/* negate positive group ids, Coda groups are negative numbers */
+	if (group_id > 0) {
+	    group_id = -group_id;
+	    /* assume this is the /etc/group file, force owner to System */
+	    owner = "System";
+	}
+
 	/* create group */
 	PDB_lookupByName(owner, &owner_id);
 	if (owner_id == 0) {
@@ -645,23 +667,34 @@
 		   group, owner);
 	    continue;
 	}
+	PDB_lookupById(group_id, &s);
+	if (s) {
+	    printf("Duplicate group for id %d, found both %s and %s\n",
+		   group_id, s, group);
+	    free(s);
+	    continue;
+	}
 	PDB_createGroup(group, owner_id, &create_id);
 	PDB_changeId(create_id, group_id);
 	printf("Created group %s, id %d, owner %s\n", group, group_id, owner);
     }   
 
-    /* add group members*/
+    /* Add group members. Groups can be members of another group, so that is
+     * why we needed to create all the groups first */
     rewind(groupfile);
     while (1) {
-	rc = fscanf(groupfile, "%[^:]:%*[^:]:%d:%s\n",
+	rc = fscanf(groupfile, "%[^:]:%*[^:]:%d:%[^\n]\n",
 		    group, &group_id, owner_and_members);
 	if (rc < 0) break;
 
+	if (group_id == 0) continue;
+
 	/* restore the :'s in the group name */
 	s = group; while ((s = strchr(s, '%')) != NULL) *s = ':';
 
-	/* skip the owner */
-	(void)strtok(owner_and_members, ",");
+	/* skip the owner when the group_id is negative */
+	if (group_id > 0) group_id = -group_id;
+	else		  (void)strtok(owner_and_members, ",");
 
 	/* add group members */
 	printf("Adding members to %s\n\t", group);