libuser

Clone
Source Code
GIT

#40 FILES: Don't allow duplicate IDs by default
Merged a year ago by thalman. Opened 5 years ago by jhrozek.
jhrozek/libuser id_check  into  master

file modified
+150 
@@ -832,6 +832,128 @@ 
 

  	return ret;
 

  }
 

  

+ static gboolean
 

+ lu_files_permits_duplicate_ids(struct lu_module *module)
 

+ {
 

+ 	const char *allow_id_dupes;
 

+ 

+ 	g_assert(module != NULL);
 

+ 	g_assert(module->lu_context != NULL);
 

+ 

+ 	allow_id_dupes = lu_cfg_read_single(module->lu_context,
 

+ 					    "files/allow_id_duplicates",
 

+ 					    "false");
 

+ 	if (allow_id_dupes &&
 

+ 	    strcasecmp(allow_id_dupes, "true") == 0) {
 

+ 		return TRUE;
 

+ 	}
 

+ 

+ 	return FALSE;
 

+ }
 

+ 

+ static gboolean
 

+ lu_files_mod_is_id_unique(struct lu_module *module, struct lu_ent *ent,
 

+ 			  struct lu_error **error)
 

+ {
 

+ 	id_t id_change = LU_VALUE_INVALID_ID;
 

+ 	struct lu_ent *dup_ent = NULL;
 

+ 	gboolean ret = FALSE;
 

+ 	gboolean found = FALSE;
 

+ 	const char *name_attribute;
 

+ 	const char *id_attribute;
 

+ 

+ 	g_assert(module != NULL);
 

+ 	g_assert(ent != NULL);
 

+ 	g_assert(error != NULL);
 

+ 

+ 	if (lu_files_permits_duplicate_ids(module)) {
 

+ 		return TRUE;
 

+ 	}
 

+ 

+ 	/* Get the array of names for the entity object. */
 

+ 	if (ent->type == lu_user) {
 

+ 		name_attribute = LU_USERNAME;
 

+ 		id_attribute = LU_UIDNUMBER;
 

+ 	} else if (ent->type == lu_group) {
 

+ 		name_attribute = LU_GROUPNAME;
 

+ 		id_attribute = LU_GIDNUMBER;
 

+ 	} else {
 

+ 		g_assert_not_reached();
 

+ 	}
 

+ 

+ 	id_change = lu_ent_get_first_id(ent, id_attribute);
 

+ 	if (id_change == LU_VALUE_INVALID_ID) {
 

+ 		/* The GID is not being changed, success */
 

+ 		return TRUE;
 

+ 	}
 

+ 

+ 	/* If the GID is being changed, check if there is another entry
 

+ 	 * with the same GID
 

+ 	 */
 

+ 	dup_ent = lu_ent_new();
 

+ 	if (dup_ent == NULL) {
 

+ 		return FALSE;
 

+ 	}
 

+ 

+ 	/* Get the array of names for the entity object. */
 

+ 	if (ent->type == lu_user) {
 

+ 		found = lu_files_user_lookup_id(module, id_change, dup_ent,
 

+ 						error);
 

+ 	} else if (ent->type == lu_group) {
 

+ 		found = lu_files_group_lookup_id(module, id_change, dup_ent,
 

+ 						error);
 

+ 	} else {
 

+ 		g_assert_not_reached();
 

+ 	}
 

+ 

+ 	if (found == TRUE) {
 

+ 		/* If there is, check if its original name is the same as
 

+ 		 * the original name of ent.
 

+ 		 */
 

+ 		const char *dup_name = NULL;
 

+ 		const char *ent_cur_name = NULL;
 

+ 

+ 		dup_name = lu_ent_get_first_string_current(dup_ent,
 

+ 							   name_attribute);
 

+ 		if (dup_name == NULL) {
 

+ 			lu_error_new(error, lu_error_generic,
 

+ 				     _("duplicate object has no %s attribute"),
 

+ 				     name_attribute);
 

+ 			ret = FALSE;
 

+ 			goto done;
 

+ 		}
 

+ 

+ 		ent_cur_name = lu_ent_get_first_string_current(ent,
 

+ 							       name_attribute);
 

+ 		if (ent_cur_name == NULL) {
 

+ 			lu_error_new(error, lu_error_generic,
 

+ 				     _("original object has no %s attribute"),
 

+ 				     name_attribute);
 

+ 			ret = FALSE;
 

+ 			goto done;
 

+ 		}
 

+ 

+ 		/* Another entry already has the same ID we're attempting to
 

+ 		 * set, * this can only work if we are also renaming the group
 

+ 		 * to the * duplicate name or if we're changing 'self'
 

+ 		 */
 

+ 		if (strcmp(dup_name, ent_cur_name) != 0) {
 

+ 			lu_error_new(error, lu_error_id_used,
 

+ 				     _("ID %lu already in use by %s"),
 

+ 				     (unsigned long) id_change, dup_name);
 

+ 			ret = FALSE;
 

+ 			goto done;
 

+ 		}
 

+ 	}
 

+ 

+ 	ret = TRUE;
 

+ done:
 

+ 	if (dup_ent != NULL) {
 

+ 		lu_ent_free(dup_ent);
 

+ 	}
 

+ 	return ret;
 

+ }
 

+ 

  /* Format a single field.
 

     Return field string for g_free (). */
 

  static char *
 
@@ -1071,6 +1193,13 @@ 
 

  lu_files_user_add(struct lu_module *module, struct lu_ent *ent,
 

  		  struct lu_error **error)
 

  {
 

+ 	gboolean ret = FALSE;
 

+ 

+ 	ret = lu_files_mod_is_id_unique(module, ent, error);
 

+ 	if (ret == FALSE) {
 

+ 		return FALSE;
 

+ 	}
 

+ 

  	return generic_add(module, suffix_passwd, format_passwd,
 

  			   G_N_ELEMENTS(format_passwd), ent, error);
 

  }
 
@@ -1112,6 +1241,13 @@ 
 

  lu_files_group_add(struct lu_module *module, struct lu_ent *ent,
 

  		   struct lu_error **error)
 

  {
 

+ 	gboolean ret = FALSE;
 

+ 

+ 	ret = lu_files_mod_is_id_unique(module, ent, error);
 

+ 	if (ret == FALSE) {
 

+ 		return FALSE;
 

+ 	}
 

+ 

  	return generic_add(module, suffix_group, format_group,
 

  			   G_N_ELEMENTS(format_group), ent, error);
 

  }
 
@@ -1262,6 +1398,13 @@ 
 

  lu_files_user_mod(struct lu_module *module, struct lu_ent *ent,
 

  		  struct lu_error **error)
 

  {
 

+ 	gboolean ret = FALSE;
 

+ 

+ 	ret = lu_files_mod_is_id_unique(module, ent, error);
 

+ 	if (ret == FALSE) {
 

+ 		return FALSE;
 

+ 	}
 

+ 

  	return generic_mod(module, suffix_passwd, format_passwd,
 

  			   G_N_ELEMENTS(format_passwd), ent, error);
 

  }
 
@@ -1271,6 +1414,13 @@ 
 

  lu_files_group_mod(struct lu_module *module, struct lu_ent *ent,
 

  		   struct lu_error **error)
 

  {
 

+ 	gboolean ret = FALSE;
 

+ 

+ 	ret = lu_files_mod_is_id_unique(module, ent, error);
 

+ 	if (ret == FALSE) {
 

+ 		return FALSE;
 

+ 	}
 

+ 

  	return generic_mod(module, suffix_group, format_group,
 

  			   G_N_ELEMENTS(format_group), ent, error);
 

  }

file modified
+61 -5 
@@ -278,6 +278,15 @@ 
 

              e[field] = str(e[field][0]) + '\nx'
 

              self.assertRaises(RuntimeError, self.a.addUser, e, False, False)
 

  

+     def testUserAdd10(self):
 

+         # Adding a duplicate user UID
 

+         e = self.a.initUser('user6_10')
 

+         self.a.addUser(e, False, False)
 

+ 

+         e2 = self.a.initUser('user6_10_2')
 

+         e2[libuser.UIDNUMBER] = e[libuser.UIDNUMBER]
 

+         self.assertRaises(RuntimeError, self.a.addUser, e2, False, False)
 

+ 

      def testUserMod1(self):
 

          # A minimal case
 

          e = self.a.initUser('user7_1')
 
@@ -301,8 +310,8 @@ 
 

          e[libuser.USERNAME] = 'user7_2username'
 

          self.assertNotEqual(e[libuser.USERPASSWORD], ['!!pwuser7_2'])
 

          e[libuser.USERPASSWORD] = '!!pwuser7_2'
 

-         self.assertNotEqual(e[libuser.UIDNUMBER], [4237])
 

-         e[libuser.UIDNUMBER] = 4237
 

+         self.assertNotEqual(e[libuser.UIDNUMBER], [4238])
 

+         e[libuser.UIDNUMBER] = 4238
 

          self.assertNotEqual(e[libuser.GIDNUMBER], [3742])
 

          e[libuser.GIDNUMBER] = 3742
 

          self.assertNotEqual(e[libuser.GECOS], ['Full Name,Office,1234,4321'])
 
@@ -336,7 +345,7 @@ 
 

          self.assertIsNotNone(e)
 

          self.assertEqual(e[libuser.USERNAME], ['user7_2username'])
 

          self.assertEqual(e[libuser.USERPASSWORD], ['!!pwuser7_2'])
 

-         self.assertEqual(e[libuser.UIDNUMBER], [4237])
 

+         self.assertEqual(e[libuser.UIDNUMBER], [4238])
 

          self.assertEqual(e[libuser.GIDNUMBER], [3742])
 

          self.assertEqual(e[libuser.GECOS], ['Full Name,Office,1234,4321'])
 

          self.assertEqual(e[libuser.HOMEDIRECTORY], ['/home/user7_2home'])
 
@@ -457,6 +466,21 @@ 
 

              e[field] = str(e[field][0]) + '\nx'
 

              self.assertRaises(RuntimeError, self.a.modifyUser, e, False)
 

  

+     def testUserMod9(self):
 

+         # Attempt to modify to create UID duplicates
 

+         e = self.a.initUser('user7_9')
 

+         self.a.addUser(e, False, False)
 

+         e_uid = e[libuser.UIDNUMBER]
 

+         del e
 

+ 

+         e = self.a.initUser('user7_9_2')
 

+         self.a.addUser(e, False, False)
 

+         del e
 

+ 

+         e = self.a.lookupUserByName('user7_9_2')
 

+         e[libuser.UIDNUMBER] = e_uid
 

+         self.assertRaises(RuntimeError, self.a.modifyUser, e, False)
 

+ 

      def testUserDel(self):
 

          e = self.a.initUser('user8_1')
 

          self.a.addUser(e, False, False)
 
@@ -946,6 +970,19 @@ 
 

                  e[field] = field + '\nx'
 

              self.assertRaises(RuntimeError, self.a.addGroup, e)
 

  

+     def testGroupAdd7(self):
 

+         # Adding a duplicate group GID
 

+         e = self.a.initGroup('group21_7')
 

+         e[libuser.MEMBERNAME] = ['group21_7member1', 'group21_7member2']
 

+         self.a.addGroup(e)
 

+         e_gid = e[libuser.GIDNUMBER]
 

+         del e
 

+ 

+         e_dup = self.a.initGroup('group21_7_dup')
 

+         self.assertNotEqual(e_dup[libuser.GIDNUMBER], [e_gid])
 

+         e_dup[libuser.GIDNUMBER] = e_gid
 

+         self.assertRaises(RuntimeError, self.a.modifyGroup, e_dup)
 

+ 

      def testGroupMod1(self):
 

          # A minimal case
 

          e = self.a.initGroup('group22_1')
 
@@ -971,7 +1008,7 @@ 
 

          self.assertNotEqual(e[libuser.GROUPPASSWORD], ['!!grgroup22_2'])
 

          e[libuser.GROUPPASSWORD] = '!!grgroup22_2'
 

          self.assertNotEqual(e[libuser.GIDNUMBER], [4237])
 

-         e[libuser.GIDNUMBER] = 4237
 

+         e[libuser.GIDNUMBER] = 4238
 

          v = sorted(e[libuser.MEMBERNAME])
 

          self.assertNotEqual(v, ['group22_2member1', 'group22_2member3'])
 

          e[libuser.MEMBERNAME] = ['group22_2member1', 'group22_2member3']
 
@@ -986,7 +1023,7 @@ 
 

          self.assertIsNotNone(e)
 

          self.assertEqual(e[libuser.GROUPNAME], ['group22_2groupname'])
 

          self.assertEqual(e[libuser.GROUPPASSWORD], ['!!grgroup22_2'])
 

-         self.assertEqual(e[libuser.GIDNUMBER], [4237])
 

+         self.assertEqual(e[libuser.GIDNUMBER], [4238])
 

          v = e[libuser.MEMBERNAME]
 

          v.sort()
 

          self.assertEqual(v, ['group22_2member1', 'group22_2member3'])
 
@@ -1077,6 +1114,25 @@ 
 

                  e[field] = field + '\nx'
 

              self.assertRaises(RuntimeError, self.a.modifyGroup, e)
 

  

+     def testGroupMod8(self):
 

+         # GID duplicate tests
 

+         e = self.a.initGroup('group22_8')
 

+         e[libuser.MEMBERNAME] = ['group22_8member1', 'group22_8member2']
 

+         self.a.addGroup(e)
 

+         e_gid = e[libuser.GIDNUMBER]
 

+         del e
 

+ 

+         e_dup = self.a.initGroup('group22_8_2')
 

+         e_dup[libuser.MEMBERNAME] = ['group22_8member1', 'group22_8member2']
 

+         self.a.addGroup(e_dup)
 

+         del e_dup
 

+ 

+         # Changing GID to e's should not work
 

+         e_dup = self.a.lookupGroupByName('group22_8_2')
 

+         self.assertNotEqual(e_dup[libuser.GIDNUMBER], [e_gid])
 

+         e_dup[libuser.GIDNUMBER] = e_gid
 

+         self.assertRaises(RuntimeError, self.a.modifyGroup, e_dup)
 

+ 

      def testGroupDel(self):
 

          e = self.a.initGroup('group23_1')
 

          self.a.addGroup(e)

Merges:
https://pagure.io/libuser/issue/39

Changes the libuser default to disallow duplicate IDs when adding or modifying users and groups.

Also adds test cases to test the new behaviour and fixes two existingtest which relied on the old behaviour.

Commit 8349f85 fixes this pull-request

Pull-Request has been merged by thalman
a year ago

Pull-Request has been merged by thalman
a year ago
Metadata
None
No Tags
Changes Summary 2
+150 -0
file changed
modules/files.c
+61 -5
file changed
tests/files_test.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.