danderson@tigris.org writes:
> --- trunk/subversion/include/svn_config.h (original)
> +++ trunk/subversion/include/svn_config.h Mon Aug 29 19:50:22 2005
> @@ -103,6 +103,7 @@
> #define SVN_CONFIG_OPTION_AUTH_ACCESS "auth-access"
> #define SVN_CONFIG_OPTION_PASSWORD_DB "password-db"
> #define SVN_CONFIG_OPTION_REALM "realm"
> +#define SVN_CONFIG_OPTION_AUTHZ_DB "authz-db"
>
> /* For repository password database */
> #define SVN_CONFIG_SECTION_USERS "users"
>
> Modified: trunk/subversion/libsvn_repos/repos.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_repos/repos.c?rev=15982&p1=trunk/subversion/libsvn_repos/repos.c&p2=trunk/subversion/libsvn_repos/repos.c&r1=15981&r2=15982
> ==============================================================================
> --- trunk/subversion/libsvn_repos/repos.c (original)
> +++ trunk/subversion/libsvn_repos/repos.c Mon Aug 29 19:50:22 2005
> @@ -1392,6 +1392,20 @@
> APR_EOL_STR
> "# password-db = passwd"
> APR_EOL_STR
> + "### The authz-db option controls the location of the authorization"
> + APR_EOL_STR
> + "### rules for path-based access control. Unless you specify a path"
> + APR_EOL_STR
> + "### starting with a /, the file's location is relative to the conf"
> + APR_EOL_STR
> + "### directory. If you don't specify an authz-db, no path-based access"
> + APR_EOL_STR
> + "### control is done."
> + APR_EOL_STR
> + "### Uncomment the line below to use the default authorization file."
> + APR_EOL_STR
> + "# authz-db = authz"
> + APR_EOL_STR
> "### This option specifies the authentication realm of the repository."
> APR_EOL_STR
> "### If two repositories have the same authentication realm, they should"
If we're going to have an "SVN_CONFIG_OPTION_AUTHZ_DB" #define, why
not splice it in above, wherever you currently have the hardcoded
string "authz-db"?
> --- trunk/subversion/libsvn_repos/repos.h (original)
> +++ trunk/subversion/libsvn_repos/repos.h Mon Aug 29 19:50:22 2005
> @@ -70,6 +70,7 @@
> /* In the repository conf directory, look for these files. */
> #define SVN_REPOS__CONF_SVNSERVE_CONF "svnserve.conf"
> #define SVN_REPOS__CONF_PASSWD "passwd"
> +#define SVN_REPOS__CONF_AUTHZ "authz"
>
> /* The Repository object, created by svn_repos_open() and
> svn_repos_create(), allocated in POOL. */
As I think was discussed on the list, the comment above those three
doesn't quite apply to all of them. I can't remember if we decided
that that shouldn't be addressed in this commit, though; just wanted
to make sure it didn't get forgotten.
(If you don't plan/want to do anything about it, that's fine, just let
me know so I can :-) ).
> --- trunk/subversion/svnserve/serve.c (original)
> +++ trunk/subversion/svnserve/serve.c Mon Aug 29 19:50:22 2005
> @@ -47,6 +47,8 @@
> svn_fs_t *fs; /* For convenience; same as svn_repos_fs(repos) */
> svn_config_t *cfg; /* Parsed repository svnserve.conf */
> svn_config_t *pwdb; /* Parsed password database */
> + svn_authz_t *authzdb; /* Parsed authz rules */
> + const char *authz_repos_name; /* The name of the repository */
> const char *realm; /* Authentication realm */
> const char *repos_url; /* URL to base of repository */
> const char *fs_path; /* Decoded base path inside repository */
> @@ -102,6 +104,73 @@
>
> /* --- AUTHENTICATION AND AUTHORIZATION FUNCTIONS --- */
>
> +/* Set *ALLOWED to TRUE if PATH is accessible in the REQUIRED mode to
> + the user described in BATON according to the authz rules in BATON.
> + Use POOL for temporary allocations only. If no authz rules are
> + present in BATON, grant access by default. */
> +static svn_error_t *authz_check_access(svn_boolean_t *allowed,
> + const char *path,
> + svn_repos_authz_access_t required,
> + server_baton_t *b,
> + apr_pool_t *pool)
The parameter's name is actually B, not BATON :-).
(I don't have a problem with calling it 'b', but if it's going to make
the doc string more awkward to write, then I'd say change the
variable's name to 'baton'.)
> +{
> + /* If authz cannot be performed, grant access. This is NOT the same
> + as the default policy when authz is performed on a path with no
> + rules. In the latter case, the default is to deny access, and is
> + set by svn_repos_authz_check_access. */
> + if (!b->authzdb || !path)
> + {
> + *allowed = TRUE;
> + return SVN_NO_ERROR;
> + }
> +
> + /* If the authz request is for the empty path (ie. ""), replace it
> + with the root path. This happens because of stripping done at
> + various levels in svnserve that remove the leading / on an
> + absolute path. Passing such a malformed path to the authz
> + routines throws them into an infinite loop and makes them miss
> + ACLs. */
> + if (*path == '\0')
> + path = "/";
> +
> + return svn_repos_authz_check_access(b->authzdb, b->authz_repos_name,
> + path, b->user, required,
> + allowed, pool);
> +}
The code still looks good the second time around. That's comforting! :-)
> +/* Set *ALLOWED to TRUE if PATH is readable by the user described in
> + * BATON. Use POOL for temporary allocations only. ROOT is not used.
> + * Implements the svn_repos_authz_func_t interface.
> + */
> +static svn_error_t *authz_check_access_cb(svn_boolean_t *allowed,
> + svn_fs_root_t *root,
> + const char *path,
> + void *baton,
> + apr_pool_t *pool)
(I notice that this time it's BATON :-) .)
Thanks for the "Implements..." line, here and elsewhere.
> +/* Check that the client has the REQUIRED access by consulting the
> + * authentication and authorization states stored in BATON. If the
> + * client does not have the required access credentials, attempt to
> + * authenticate the client to get that access, using CONN for
> + * communication.
> + *
> + * This function is supposed to be called to handle the authentication
> + * half of a standard svn protocol reply. If an error is returned, it
> + * probably means that the server can terminate the client connection
> + * with an apologetic error, as it implies an authentication failure.
> + *
> + * PATH and NEEDS_USERNAME are passed along to lookup_access, their
> + * behaviour is documented there.
> + */
> +static svn_error_t *must_have_access(svn_ra_svn_conn_t *conn,
> + apr_pool_t *pool,
> + server_baton_t *b,
> + svn_repos_authz_access_t required,
> + const char *path,
> + svn_boolean_t needs_username)
Again, B not BATON.
It's unusual to put POOL anywhere other than the end of the parameter
list; was there a reason? (Sorry, didn't notice last time around.)
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 30 06:47:39 2005