Re: [PATCH v2] id: add feature to accept multiple usernames

[Achilles Gaikwad]

Hello,

Any thoughts about this?

Best,
- Achilles


On Tue, Jul 24, 2018 at 1:09 AM Achilles Gaikwad <address@hidden> wrote:
>
> Adds a feature to id command to accept multiple usernames and/or uid
> as arguments.
>
> $ id root nobody
> uid=0(root) gid=0(root) groups=0(root)
> uid=99(nobody) gid=99(nobody) groups=99(nobody)
>
> * src/id.c (main): make varibles opt_zero, just_group_list,
> just_group, use_real, just_user global to be used in a new
> function.
> (print_stuff): new function that will print user and group information
> for the specified USER.
> When using -G option delimit each record with two consequent NULs.
> Restructure the code in the file to have global variables followed by
> functions to make the file look pretty.
> * tests/id/zero.sh: Add test cases to check the usage of -z option with
> multiple users.
> * doc/coreutils.texi: add minor documentation change to reflect the
> number of inputs that may be given.
>
> Signed-off-by: Achilles Gaikwad <address@hidden>
> ---
>  doc/coreutils.texi |   2 +-
>  src/id.c           | 171 +++++++++++++++++++++++++--------------------
>  tests/id/zero.sh   |  34 +++++++++
>  3 files changed, 131 insertions(+), 76 deletions(-)
>
> diff --git a/doc/coreutils.texi b/doc/coreutils.texi
> index 10fd023d8..f2d58b46d 100644
> --- a/doc/coreutils.texi
> +++ b/doc/coreutils.texi
> @@ -15200,7 +15200,7 @@ logins, groups, and so forth.
>  running it if no user is specified.  Synopsis:
>
>  @example
> -id [@var{option}]@dots{} [@var{user}]
> +id [@var{option}]@dots{} [@var{user}]@dots{}
>  @end example
>
>  @var{user} can be either a user ID or a name, with name look-up
> diff --git a/src/id.c b/src/id.c
> index be0758059..e60803894 100644
> --- a/src/id.c
> +++ b/src/id.c
> @@ -43,10 +43,20 @@
>
>  /* If nonzero, output only the SELinux context.  */
>  static bool just_context = 0;
> -
> -static void print_user (uid_t uid);
> -static void print_full_info (const char *username);
> -
> +/* If true, delimit entries with NUL characters, not whitespace */
> +bool opt_zero = false;
> +/* If true, output the list of all group IDs. -G */
> +bool just_group_list = false;
> +/* If true, output only the group ID(s). -g */
> +bool just_group = false;
> +/* If true, output real UID/GID instead of default effective UID/GID. -r */
> +bool use_real = false;
> +/* If true, output only the user ID(s). -u */
> +bool just_user = false;
> +/* True unless errors have been encountered.  */
> +static bool ok = true;
> +/* If true, we are using multiple users. Terminate -G with double NUL. */
> +static bool multiple_users = false;
>  /* If true, output user/group name instead of ID number. -n */
>  static bool use_name = false;
>
> @@ -54,13 +64,14 @@ static bool use_name = false;
>  static uid_t ruid, euid;
>  static gid_t rgid, egid;
>
> -/* True unless errors have been encountered.  */
> -static bool ok = true;
> -
>  /* The SELinux context.  Start with a known invalid value so print_full_info
>     knows when 'context' has not been set to a meaningful value.  */
>  static char *context = NULL;
>
> +static void print_user (uid_t uid);
> +static void print_full_info (const char *username);
> +static void print_stuff(const char *pw_name);
> +
>  static struct option const longopts[] =
>  {
>    {"context", no_argument, NULL, 'Z'},
> @@ -82,9 +93,9 @@ usage (int status)
>      emit_try_help ();
>    else
>      {
> -      printf (_("Usage: %s [OPTION]... [USER]\n"), program_name);
> +      printf (_("Usage: %s [OPTION]... [USER]...\n"), program_name);
>        fputs (_("\
> -Print user and group information for the specified USER,\n\
> +Print user and group information for the each specified USER,\n\
>  or (when USER omitted) for the current user.\n\
>  \n"),
>               stdout);
> @@ -116,18 +127,8 @@ main (int argc, char **argv)
>    int optc;
>    int selinux_enabled = (is_selinux_enabled () > 0);
>    bool smack_enabled = is_smack_enabled ();
> -  bool opt_zero = false;
>    char *pw_name = NULL;
>
> -  /* If true, output the list of all group IDs. -G */
> -  bool just_group_list = false;
> -  /* If true, output only the group ID(s). -g */
> -  bool just_group = false;
> -  /* If true, output real UID/GID instead of default effective UID/GID. -r */
> -  bool use_real = false;
> -  /* If true, output only the user ID(s). -u */
> -  bool just_user = false;
> -
>    initialize_main (&argc, &argv);
>    set_program_name (argv[0]);
>    setlocale (LC_ALL, "");
> @@ -185,11 +186,6 @@ main (int argc, char **argv)
>      }
>
>    size_t n_ids = argc - optind;
> -  if (1 < n_ids)
> -    {
> -      error (0, 0, _("extra operand %s"), quote (argv[optind + 1]));
> -      usage (EXIT_FAILURE);
> -    }
>
>    if (n_ids && just_context)
>      die (EXIT_FAILURE, 0,
> @@ -228,29 +224,44 @@ main (int argc, char **argv)
>          die (EXIT_FAILURE, 0, _("can't get process context"));
>      }
>
> -  if (n_ids == 1)
> -    {
> -      struct passwd *pwd = NULL;
> -      const char *spec = argv[optind];
> -      /* Disallow an empty spec here as parse_user_spec() doesn't
> -         give an error for that as it seems it's a valid way to
> -         specify a noop or "reset special bits" depending on the system.  */
> -      if (*spec)
> -        {
> -          if (parse_user_spec (spec, &euid, NULL, NULL, NULL) == NULL)
> -            {
> -              /* parse_user_spec will only extract a numeric spec,
> -                 so we lookup that here to verify and also retrieve
> -                 the PW_NAME used subsequently in group lookup.  */
> -              pwd = getpwuid (euid);
> -            }
> -        }
> -      if (pwd == NULL)
> -        die (EXIT_FAILURE, 0, _("%s: no such user"), quote (spec));
> -      pw_name = xstrdup (pwd->pw_name);
> -      ruid = euid = pwd->pw_uid;
> -      rgid = egid = pwd->pw_gid;
> -    }
> +  if (n_ids >= 1)
> +  {
> +      multiple_users = n_ids > 1 ? true : false;
> +      /* Changing the value of n_ids to the last index in the array where we
> +         have the last possible user id. This helps us because we don't have
> +         to declare a different variable to keep a track of where the last 
> username
> +         lies in argv[]. */
> +      n_ids += optind;
> +      /* For each username/userid to get its pw_name field */
> +      for (; optind < n_ids; optind++)
> +      {
> +          struct passwd *pwd = NULL;
> +          const char *spec = argv[optind];
> +          /* Disallow an empty spec here as parse_user_spec() doesn't
> +             give an error for that as it seems it's a valid way to
> +             specify a noop or "reset special bits" depending on the system. 
>  */
> +          if (*spec) {
> +              if (parse_user_spec(spec, &euid, NULL, NULL, NULL) == NULL)
> +              {
> +                  /* parse_user_spec will only extract a numeric spec,
> +                     so we lookup that here to verify and also retrieve
> +                     the PW_NAME used subsequently in group lookup.  */
> +                  pwd = getpwuid(euid);
> +              }
> +          }
> +          if (pwd == NULL)
> +          {
> +            error (0, errno, _("%s: no such user"), quote (argv[optind]));
> +            ok &= false;
> +            continue;
> +          }
> +          pw_name = xstrdup(pwd->pw_name);
> +          ruid = euid = pwd->pw_uid;
> +          rgid = egid = pwd->pw_gid;
> +          print_stuff(pw_name);
> +          IF_LINT (free (pw_name));
> +      }
> +  }
>    else
>      {
>        /* POSIX says identification functions (getuid, getgid, and
> @@ -289,34 +300,10 @@ main (int argc, char **argv)
>            if (rgid == NO_GID && errno)
>              die (EXIT_FAILURE, errno, _("cannot get real GID"));
>          }
> +        print_stuff(pw_name);
> +        IF_LINT (free (pw_name));
>      }
>
> -  if (just_user)
> -    {
> -      print_user (use_real ? ruid : euid);
> -    }
> -  else if (just_group)
> -    {
> -      if (!print_group (use_real ? rgid : egid, use_name))
> -        ok = false;
> -    }
> -  else if (just_group_list)
> -    {
> -      if (!print_group_list (pw_name, ruid, rgid, egid, use_name,
> -                             opt_zero ? '\0' : ' '))
> -        ok = false;
> -    }
> -  else if (just_context)
> -    {
> -      fputs (context, stdout);
> -    }
> -  else
> -    {
> -      print_full_info (pw_name);
> -    }
> -  putchar (opt_zero ? '\0' : '\n');
> -
> -  IF_LINT (free (pw_name));
>    return ok ? EXIT_SUCCESS : EXIT_FAILURE;
>  }
>
> @@ -356,7 +343,7 @@ print_user (uid_t uid)
>          {
>            error (0, 0, _("cannot find name for user ID %s"),
>                   uidtostr (uid));
> -          ok = false;
> +          ok &= false;
>          }
>      }
>
> @@ -415,7 +402,7 @@ print_full_info (const char *username)
>                   quote (username));
>          else
>            error (0, errno, _("failed to get groups for the current 
> process"));
> -        ok = false;
> +        ok &= false;
>          return;
>        }
>
> @@ -438,3 +425,37 @@ print_full_info (const char *username)
>    if (context)
>      printf (_(" context=%s"), context);
>  }
> +
> +/* Print information about the user based on the arguments passed. */
> +
> +static void
> +print_stuff(const char *pw_name)
> +{
> +  if (just_user)
> +      print_user (use_real ? ruid : euid);
> +
> +  /* print_group and print_group_lists functions return true on successful
> +     execution but false if something goes wrong. We then AND this value with
> +     the current value of 'ok' because we want to know if one of the previous
> +     users faced a problem in these functions. This value of 'ok' is later 
> used
> +     to understand what status program should exit with. */
> +  else if (just_group)
> +      ok &= print_group (use_real ? rgid : egid, use_name);
> +  else if (just_group_list)
> +      ok &= print_group_list (pw_name, ruid, rgid, egid,
> +               use_name, opt_zero ? '\0' : ' ');
> +  else if (just_context)
> +      fputs (context, stdout);
> +  else
> +      print_full_info (pw_name);
> +  /* When printing records for more than 1 user, at the end of groups
> +     of each user terminate the record with two consequent NUL characters
> +     to make parsing and distinguishing between two records possible. */
> + if (opt_zero && just_group_list && multiple_users)
> +    {
> +      putchar('\0');
> +      putchar('\0');
> +    }
> +  else
> +      putchar (opt_zero ? '\0' : '\n');
> +}
> diff --git a/tests/id/zero.sh b/tests/id/zero.sh
> index f183e18f8..7a94edde1 100755
> --- a/tests/id/zero.sh
> +++ b/tests/id/zero.sh
> @@ -63,4 +63,38 @@ printf '\n' >> out || framework_failure_
>  tr '\0' ' ' < out > out2 || framework_failure_
>  compare exp out2 || fail=1
>
> +# multiuser testing with -z
> +# test if the options work, these tests should pass if the above tests
> +# do.
> +
> +for o in g gr u ur ; do
> +  for n in '' n ; do
> +    id -${o}${n}  $users >> tmp1 ||
> +      { test $? -ne 1 || test -z "$n" && fail=1; }
> +    id -${o}${n}z $users  > tmp2 ||
> +      { test $? -ne 1 || test -z "$n" && fail=1; }
> +    tr '\0' '\n' < tmp2 >> tmp3
> +  done
> +done
> +compare tmp1 tmp3 || fail=1
> +
> +# Seperate checks when we are testing for multiple users && -G.
> +# This is done because we terminate the records with two consequent
> +# NULs instead of the regular single NUL.
> +
> +for o in G Gr ; do
> +  for n in '' n ; do
> +    id -${o}${n}  $users >> gtmp1 ||
> +      { test $? -ne 1 || test -z "$n" && fail=1; }
> +    id -${o}${n}z $users  > gtmp2 ||
> +      { test $? -ne 1 || test -z "$n" && fail=1; }
> +    # we replace all NULs with spaces, the result we get is there are two
> +    # consequent spaces instead of two consequent NUL's, we pass this to sed
> +    # to replace more than 1 space with a newline. This is ideally where a 
> new
> +    # line should be. This should make the output similar to without -z.
> +    tr '\0' ' ' < gtmp2 | sed 's/ \+ /\n/g' >> gtmp3
> +  done
> +done
> +compare gtmp1 gtmp3 || fail=1
> +
>  Exit $fail
> --
> 2.17.1
>
>