Feature #6477

Redmine.pm: effective redmine svn rights

Added by Jean Van Dooren about 9 years ago. Updated about 9 years ago.

Status:ClosedStart date:2010-09-23
Priority:NormalDue date:
Assignee:-% Done:

100%

Category:-
Target version:-
Resolution:

Description

Hi,

i modified the redmine.pm file in order to (try to) match Redmine rights and SVN methods.
If you use curent Redmine.pm file, you'll see that if you check the "Browse repository" right in Redmine for a user (role->user), the user can do everything in SVN.
Even if you don't check commit access into Redmine...

Redmine rights on SVN are:
  • Manage repository
  • View changesets
  • Browse repository
  • Commit access

SVN methods are:
OPTIONS, PROPFIND, GET, REPORT, MKACTIVITY, PROPPATCH, PUT, CHECKOUT, MKCOL,MOVE, COPY, DELETE, LOCK, UNLOCK, MERGE

you must add after
my @directives { }; (on row 146)

my %authorizations = (
    'GET', 'browse_repository',
    'PROPFIND', 'browse_repository',
    'REPORT', 'browse_repository',
    'OPTIONS', 'browse_repository',
    'MKACTIVITY','commit_access',
    'PROPPATCH','commit_access',
    'PUT', 'commit_access',
    'CHECKOUT','commit_access',
    'MKCOL','commit_access',
    'MOVE','commit_access',
    'COPY','commit_access',
    'DELETE','commit_access',
    'LOCK','commit_access',
    'UNLOCK','commit_access',
    'MERGE','commit_access'
);

and then replace row 328:
if ($hashed_password eq $pass_digest && ((defined $read_only_methods{$method} && $permissions =~ /:browse_repository/) || $permissions =~ /:commit_access/) ) {

by
if ($hashed_password eq $pass_digest && ( $permissions =~ m/($authorizations{$method})/ ) ) {

and at row 387

$ret = 1 if ($ldap->authenticate($redmine_user, $redmine_pass) && ((defined $read_only_methods{$method} && $permissions =~ /:browse_repository/) || $permissions =~ /:commit_access/));

by
$ret = 1 if ($ldap->authenticate($redmine_user, $redmine_pass) && $permissions =~ m/($authorizations{$method})/);

I'm sure there are other modifications to do (add rights in cache) and a better way to do that.

I hope it'll help others,
Best regards,
Jean

History

#1 Updated by Felix Schäfer about 9 years ago

Jean Van Dooren wrote:

i modified the redmine.pm file in order to (try to) match Redmine rights and SVN methods.
If you use curent Redmine.pm file, you'll see that if you check the "Browse repository" right in Redmine for a user (role->user), the user can do everything in SVN.
Even if you don't check commit access into Redmine…

I have the current Redmine.pm and users with no commit access can't commit on my installation, and this has also been verified by others. I don't see how your proposed patch fundamentally changes the workings of Redmine.pm either. What problem did you have that triggered you to make those changes?

#2 Updated by Jean Van Dooren about 9 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

Hi Felix,

Your are absolutely right. I restart some tests with orginial Redmine.pm file.
  1. Browse repository option in Redmine --> only browse SVN repository allowed
  2. Commit access option in Redmine --> full SVN access allowed

I don't know why we came on that conclusion - maybe our Apache configuration file was not good in the begining of our tests. Maybe the RedmineCacheCredentials disturbed our tests.

Sorry for this message - you can delete this post.

Thank you for your contribution in Redmine,
Best regards,
Jean

#3 Updated by Jean Van Dooren about 9 years ago

Hi Felix,

I made one more test. I finally find the problem but my modification does not correct it...

If I enable RedmineCacheCredsMax in Apache configuration file, I can do anything on SVN. All you need is to be member of the project.

I think RedmineCache should only keep user and password, rights on project must be checked on each access.

Best regards,
Jean

#4 Updated by Jean Van Dooren about 9 years ago

I modified Remdine.pm file with this:
Remove code between line 310 and 313:

  if ($cfg->{RedmineCacheCredsMax}) {
    $usrprojpass = $cfg->{RedmineCacheCreds}->get($redmine_user.":".$project_id);
    return 1 if (defined $usrprojpass and ($usrprojpass eq $pass_digest));
  }

Remove line 327 and 341:

 my $method = $r->method;

Insert modified code at line 319:

while (my ($hashed_password, $auth_source_id, $permissions) = $sth->fetchrow_array) {
     my $method = $r->method;
     if ($cfg->{RedmineCacheCredsMax}) {
         $usrprojpass = $cfg->{RedmineCacheCreds}->get($redmine_user.":".$project_id);
         return 1 if ((defined $usrprojpass and ($usrprojpass eq $pass_digest)) && ((defined $read_only_methods{$method} && $permissions =~ /:browse_repository/) || $permissions =~ /:commit_access/));
     }
     unless ($auth_source_id) {
     my $method = $r->method;

This will check user permission even if password is not verified for each action.

#5 Updated by Felix Schäfer about 9 years ago

  • Status changed from Resolved to Closed

Thanks for the feedback Jean.

Could you please open a new issue for this, and provide a diff rather than a "description" of the cahnges. Thanks!

#6 Updated by Hans Schmidt about 9 years ago

Hey Jean,

i tried to change the Redmine.pm.
But somewhere i have an fault because the apache will not start. No error in logfile.

Plz upload the diff.

Best regards,
Hans

#7 Updated by Terence Mill about 9 years ago

Felix, plz don't close the ticket as long as there is no complete solution (diff). We tried to rebuild the "description" and afterwards the apache dopn't even start.
The Bug is still here, or can anybody provide a tested diff?

Felix Schäfer wrote:

Thanks for the feedback Jean.

Could you please open a new issue for this, and provide a diff rather than a "description" of the cahnges. Thanks!

#8 Updated by Felix Schäfer about 9 years ago

Terence Mill wrote:

Felix, plz don't close the ticket as long as there is no complete solution (diff). We tried to rebuild the "description" and afterwards the apache dopn't even start.
The Bug is still here, or can anybody provide a tested diff?

The originally reported bug is fixed in current redmine. If you have found other problems, for example with the permission handling when using credential caching, then please open a new issue.

#9 Updated by Terence Mill about 9 years ago

You say its fixed .. but in which version? I thought that the redmine.pm is no part of the core, moreover a deeper svn integration Patch.

We had orginally an svn check-in performance problem without the cache, cause for every folder the rights are checked again.
We will give it a try with the cache enabled now, an hope that will fix the perfoemance issue.

#10 Updated by Felix Schäfer about 9 years ago

Terence Mill wrote:

You say its fixed .. but in which version? I thought that the redmine.pm is no part of the core, moreover a deeper svn integration Patch.

HINT The originally reported issue being Redmine.pm not always respecting read and/or commit accesses correctly has been fixed in the 2 last commits listed there. And Redmine.pm is part of core, though probably not the part getting the most love in it. As I have already said twice, if there is a problem with the caching mechanism, open a new issue.

We had orginally an svn check-in performance problem without the cache, cause for every folder the rights are checked again.
We will give it a try with the cache enabled now, an hope that will fix the perfoemance issue.

Try adding SVNPathAuthz off to the Location block in your apache config, this will basically only check permissions on the first level, i.e. repository level, which is enough because redmine doesn't provide finer-grained permissions.

#11 Updated by Terence Mill about 9 years ago

Ok, i understood. Tx for exercising patience ;)

Also available in: Atom PDF