Edgewall Software
Modify

Opened 17 years ago

Closed 14 years ago

Last modified 2 years ago

#4997 closed enhancement (duplicate)

Check SVN authz permissions with svn_authz_check_access

Reported by: hyugaricdeau@… Owned by: Remy Blank
Priority: normal Milestone:
Component: version control Version: 0.10.3
Severity: normal Keywords: svnauthz, swig, consider, authzsourcepolicy
Cc: peter.wihl@…, dfraser, alvaro.iradier@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

I'm just wondering if there's a good reason not to use the Subversion SWIG bindings to check access in authz files. My motivation here is that I would like to take advantage of certain authz syntax available in currently in Subversion's trunk, such as aliases, and special tokens like "$authorized" and "$anonymous".

The simplest solution, rather than reimplement those features in Python, was just to rewrite RealSubversionAuthorizer in svn_authz.py like so:

import svn.repos

class RealSubversionAuthorizer(Authorizer):

    auth_name = ''
    module_name = ''
    conf_authz = None

    def __init__(self, repos, auth_name, module_name, cfg_file):
        self.repos = repos

        # Because the username 'anonymous' doesn't mean anything
        # special to Subversion, it interprets 'anonymous' as an
        # authenticated user.
        if auth_name == 'anonymous':
            self.auth_name = None
        else:
            self.auth_name = auth_name

        self.module_name = module_name
        self.authz = svn.repos.authz_read(cfg_file, False)
                                
    def has_permission(self, path):
        if path is None:
            return True

        # svn_authz_check_access will crash your machine if the path
        # is not rooted at /, so this is crucial.
        # A trailing slash is also bad, as Subversion doesn't consider
        # that canonical.
        path = '/' + path.strip('/')

        return svn.repos.authz_check_access(self.authz, self.module_name,
                                            path, self.auth_name,
                                            svn.repos.svn_authz_read)

    # has_permission_for_changeset is unchanged

This could probably be improved here or there, but it seems to work well enough. I say 'seems' because the one downside I see is that this implementation anyways breaks the unit tests (svn.repos.read_authz() can't take a file pointer as an argument).

However, there may be more esoteric problems with doing it this way, so that's what I would like to confirm.

Thanks.

Attachments (1)

use_svn_api_for_authz_0_11_4.patch (4.1 KB ) - added by alvaro.iradier@… 15 years ago.
Use authz checking functions from SVN Swig API instead of parsing the Authz file if SVN ≥ 1.3

Download all attachments as: .zip

Change History (16)

comment:1 by hyugaricdeau@…, 17 years ago

Ah, I was just looking at the libsvn_repos API and noticed that the calls to authz_read and authz_check_access should also be passed pointers to the repository connection's memory pool. This is a simple modification to the code I posted above.

comment:2 by anonymous, 17 years ago

Cc: peter.wihl@… added

comment:3 by Matthew Good, 17 years ago

This function was added in SVN 1.3 and we're maintaining compatibility with older versions, so for now we can't use it exclusively. It may be possible to provide a compatibility layer so that it's used if available.

comment:4 by Christian Boos, 17 years ago

Keywords: consider added
Milestone: 0.12

comment:5 by hyuga <hyugaricdeau@…>, 17 years ago

Sounds reasonable. I did not know that those functions weren't available prior to SVN 1.3 (I hadn't started using it until 1.3).

At any rate, there are some nice new features for authz files waiting for inclusion in a release in their trunk. I've been making use of those features in a patched version 1.3.1, and they've been working well. I don't know when they plan to include them in a release, but I suspect they will eventually.

So, it seems to make sense to just use SVN's built-in authz functions when available, rather than bothering to write in one's own support for those features. And it doesn't make sense to make those new features available to users of older versions of Subversion either.

If you want I could submit a proper patch with such a compatability layer, or should this be held off on for now?

comment:6 by Christian Boos, 17 years ago

Well, there's unfortunately never been much attention paid to the authz part, so if you can get into it and help improving this part of the code, you're more than welcome!

(PS: also take a look at the existing issues with authz: query:?status=!closed&keywords=~authz)

comment:7 by Emmanuel Blot, 17 years ago

BTW, what is the policy with Subversion support?

AFAICT, 1.2.x series have officially reached end-of-life, so does Trac really need to support < 1.3 series for an upcoming release?

comment:8 by dfraser, 15 years ago

Cc: dfraser added

Note that there are severe memory leaks with these subversion python bindings before 1.5: svn-issue:3052

Last edited 2 years ago by Jun Omae (previous) (diff)

in reply to:  8 comment:9 by ebray, 15 years ago

Replying to dfraser:

Note that there are severe memory leaks with these subversion python bindings before 1.5: svn-issue:3052

I'm afraid I don't see how that relates to either of the API calls used in this patch. I've been using this patch for almost two years with SVN 1.3.x-1.5.x with no problems so far.

Last edited 2 years ago by Jun Omae (previous) (diff)

comment:10 by alvaro.iradier@…, 15 years ago

Attached is a patch for 0.11.4 that makes use of the subversion API for authz checking, only if svn version ≥ 1.3.

Adittionally, if automatically figures out the module name (repo_name) from the last repository path component.

Is it too much asking to have this included in 0.11.5? Or at least make it available through an option, like use_svn_authz_checking = True

by alvaro.iradier@…, 15 years ago

Use authz checking functions from SVN Swig API instead of parsing the Authz file if SVN ≥ 1.3

comment:11 by alvaro.iradier@…, 15 years ago

Cc: alvaro.iradier@… added

comment:12 by Christian Boos, 15 years ago

Keywords: svnauthz added; authz svn removed

in reply to:  description comment:13 by Remy Blank, 14 years ago

Milestone: next-major-0.1X0.13
Owner: changed from Christian Boos to Remy Blank

Replying to hyugaricdeau@…:

I'm just wondering if there's a good reason not to use the Subversion SWIG bindings to check access in authz files.

Yes, to be able to use the same mechanism for other backends, without requiring the Subversion bindings. The authz support has been rewritten in 0.12 as a permission policy. The only thing still missing is support for $authorized and $anonymous. I'll add those.

comment:14 by Remy Blank, 14 years ago

Milestone: 0.13
Resolution: duplicate
Status: newclosed

We had a ticket already for adding $anonymous and $authenticated, so I'm closing this as a duplicate of #8289.

comment:15 by Ryan J Ollos, 11 years ago

Keywords: authzsourcepolicy added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.