Nick Piper <nick.pi...@logica.com> writes:

> [[[
> Implement AuthzSVNRepoRelativeAccessFile to allow SVNParentPath to use
> a different authz configuration file for each repository.
>
> * subversion/mod_authz_svn/mod_authz_svn.c
>   (get_access_conf) Check if AuthzSVNRepoRelativeAccessFile is on, and
>   if so, load the conf/authz file from inside the repository being
>   accessed rather than one which is statically configured in the
>   Apache configuration.
>   (subreq_bypass, access_checker, check_user_id, auth_checker)
>   Recognise that it's valid not to have a AuthzSVNAccessFile if
>   AuthzSVNRepoRelativeAccessFile is used.
> ]]]

Sounds like a reasonable thing to do.

> Index: subversion/mod_authz_svn/mod_authz_svn.c
> ===================================================================
> --- subversion/mod_authz_svn/mod_authz_svn.c  (revision 1028364)
> +++ subversion/mod_authz_svn/mod_authz_svn.c  (working copy)
> @@ -50,6 +50,7 @@
>    int authoritative;
>    int anonymous;
>    int no_auth_when_anon_ok;
> +  int repo_relative_access_file;
>    const char *base_path;
>    const char *access_file;
>    const char *force_username_case;
> @@ -103,6 +104,13 @@
>                 "Set to 'On' to suppress authentication and authorization "
>                 "for requests which anonymous users are allowed to perform. "
>                 "(default is Off.)"),
> +  AP_INIT_FLAG("AuthzSVNRepoRelativeAccessFile", ap_set_flag_slot,
> +            (void *)APR_OFFSETOF(authz_svn_config_rec,
> +                                 repo_relative_access_file),

The name should be AuthzSVNReposRelativeAccessFile for consistency with
SVNReposName.

It should be an error to specify both AuthzSVNAccessFile and
AuthzSVNReposRelativeAccessFile similar to SVNPath/SVNParentPath.

Perhaps it should be a path rather than a boolean?


> +            OR_AUTHCFG,
> +            "Set to 'On' to allow the AuthzSVNAccessFile to be relative "
> +            "to the repository disk path. "
> +            "(default is Off.)"),
>    AP_INIT_TAKE1("AuthzForceUsernameCase", ap_set_string_slot,
>                  (void *)APR_OFFSETOF(authz_svn_config_rec,
>                                       force_username_case),
> @@ -119,18 +127,34 @@
>  get_access_conf(request_rec *r, authz_svn_config_rec *conf)
>  {
>    const char *cache_key = NULL;
> +  const char *access_file = NULL;
> +  const char *repos_path = NULL;

Those don't need to be initialised.

>    void *user_data = NULL;
>    svn_authz_t *access_conf = NULL;
>    svn_error_t *svn_err;
> +  dav_error *dav_err;
>    char errbuf[256];
>  
> +  if (conf->repo_relative_access_file) {
> +    dav_err = dav_svn_get_repos_path(r, conf->base_path, &repos_path);
> +    if (dav_err) {
> +      ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, dav_err->desc);
> +      return NULL;
> +    }
> +    access_file = apr_pstrcat(r->pool, repos_path, "/conf/authz", NULL);

Use svn_dirent_join_many.

> +  } else {
> +    access_file = conf->access_file;
> +  }

The whitespace is wrong, it should be

     if ( )
       {
         dav_err = 
       }

> +
> +  ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "Path to authz file is %s", 
> access_file);
> +
>    cache_key = apr_pstrcat(r->pool, "mod_authz_svn:",
> -                          conf->access_file, (char *)NULL);
> +                          access_file, (char *)NULL);
>    apr_pool_userdata_get(&user_data, cache_key, r->connection->pool);
>    access_conf = user_data;
>    if (access_conf == NULL)
>      {
> -      svn_err = svn_repos_authz_read(&access_conf, conf->access_file,
> +      svn_err = svn_repos_authz_read(&access_conf, access_file,
>                                       TRUE, r->connection->pool);
>        if (svn_err)
>          {

-- 
Philip

Reply via email to