On Wed, Apr 16, 2014 at 09:47:27PM +0400, Post Master wrote:
>
> subversion need to specify group members in the 'groups' section of
> svnaccess.conf configuration file.
> external access control system like LDAP, AD, &etc. requires to syncronize
> group
> members to svnaccess.conf (for example: http://thoughtspark.org/node/26/).
> subversion must query operating system for group members directly.
> for example: on posix systems from nss (ldap, nis, /etc/group ...). on
> windows:
> from AD or local groups.
>
> authz_posixgroup_contains_user.patch is a prototype for posix system
> (getgrnam).
>
> svnaccess.conf may be like that:
>
> [repos1:/]
> %wheel = rw
> %members.test.bla-bla-bla = r
>
> '%'-prefix means system group
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=4489
I like this idea.
What will happen if someone is already using group names that
start with '%'? I think this feature should be backwards compatible.
Perhaps we could add an option to the authz file format that enables
the new '%' syntax, and disable the new syntax by default.
Something like this:
[groups]
use_posix_groups = true
[repos1:/]
%wheel = rw
%members.test.bla-bla-bla = r
I doubt anyone has a group named 'use_posix_groups', so this should
be fairly safe. Or we could add a new [options] section for this
purpose instead:
[options]
use_posix_groups = true
What do you think?
> --- ./subversion/libsvn_repos/authz.c.orig 2013-05-04 01:21:54.000000000 +0400
> +++ ./subversion/libsvn_repos/authz.c 2014-04-06 17:18:40.000000000 +0400
> @@ -25,6 +25,7 @@
>
> #include <apr_pools.h>
> #include <apr_file_io.h>
You need to #include <sys/types.h> here, too.
> +#include <grp.h>
>
> #include "svn_hash.h"
> #include "svn_pools.h"
> @@ -197,6 +198,25 @@
> return FALSE;
> }
>
> +static svn_boolean_t
> +authz_posixgroup_contains_user(svn_config_t *cfg,
> + const char *group,
> + const char *user,
> + apr_pool_t *pool)
> +{
The 'pool' parmeter is unused.
> + struct group *grp;
> + char **gmem;
> +
> + if ((grp = getgrnam(group)) == NULL)
It would be nice if APR offered an interface to this function.
I checked but couldn't find one.
Please use getgrgid_r() instead. getgrnam isn't thread-safe.
We might want to use conditional compilation to support systems
where getgrnam_r() is not available. Can you add configure script
checks for grp.h and getgrnam_r(), and only call the getgrnam_r
function if it is available?
> + perror("getgrnam() error");
Please don't write anything to stderr.
libsvn_repos is a library, it should not use stdio.
Just return FALSE in this case.
> + else
> + for (gmem=grp->gr_mem; *gmem != NULL; gmem++)
> + if (strcmp(*gmem, user) == 0)
> + return TRUE;
> +
> + return FALSE;
> +}
> +
>
> /* Determines whether an authz rule applies to the current
> * user, given the name part of the rule's name-value pair
> @@ -242,6 +262,9 @@
> if (rule_match_string[0] == '@')
> return authz_group_contains_user(
> b->config, &rule_match_string[1], b->user, pool);
> + else if (rule_match_string[0] == '%')
> + return authz_posixgroup_contains_user(
> + b->config, &rule_match_string[1], b->user, pool);
> else if (rule_match_string[0] == '&')
> return authz_alias_is_user(
> b->config, &rule_match_string[1], b->user, pool);
Received on 2014-04-17 14:54:13 CEST