Add write control on repository from Redmine interface

  * new methods to add/remove rights in app/models/role.rb
  * some unit tests
  * add write check in Redmine.pm

To keep compatibility migration add write rights to non builtin roles
but default clean install give write access only to manager and
developer, not to reporter.

git-svn-id: http://redmine.rubyforge.org/svn/branches/nbc@1791 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/models/role.rb b/app/models/role.rb
index 6f1fb476868e7288edb4e03d5351ad34dac0cea5..6d2f643fdfdf20cc71020a7ae696eb5d4088f3b2 100644 (file)
--- a/app/models/role.rb
+++ b/app/models/role.rb
@@ -36,7 +36,7 @@ class Role < ActiveRecord::Base
   has_many :members
   acts_as_list
   
-  serialize :permissions
+  serialize :permissions, Array
   attr_protected :builtin
 
   validates_presence_of :name
@@ -49,9 +49,27 @@ class Role < ActiveRecord::Base
   end
   
   def permissions=(perms)
-    perms = perms.collect {|p| p.to_sym unless p.blank? }.compact if perms
+    perms = perms.collect {|p| p.to_sym unless p.blank? }.compact.uniq if perms
     write_attribute(:permissions, perms)
   end
+
+  def add_permission!(*perms)
+    self.permissions = [] unless permissions.is_a?(Array)
+
+    permissions_will_change!
+    perms.each do |p|
+      p = p.to_sym
+      permissions << p unless permissions.include?(p)
+    end
+    save!
+  end
+
+  def remove_permission!(*perms)
+    return unless permissions.is_a?(Array)
+    permissions_will_change!
+    perms.each { |p| permissions.delete(p.to_sym) }
+    save!
+  end
   
   def <=>(role)
     position <=> role.position

diff --git a/db/migrate/20080827185639_add_repository_write_access.rb b/db/migrate/20080827185639_add_repository_write_access.rb
new file mode 100644 (file)
index 0000000..2ea6e1e
--- /dev/null
+++ b/db/migrate/20080827185639_add_repository_write_access.rb
@@ -0,0 +1,14 @@
+class AddRepositoryWriteAccess < ActiveRecord::Migration
+
+  def self.up
+       Role.find(:all).select { |r| not r.builtin? }.each do |r|
+            r.add_permission!(:commit_access)
+       end
+  end
+
+  def self.down
+       Role.find(:all).select { |r| not r.builtin? }.each do |r|
+            r.remove_permission!(:commit_access)
+       end
+  end
+end

diff --git a/extra/svn/Redmine.pm b/extra/svn/Redmine.pm
index 2619196c7a1e77c32251f30c70f3e9047209af26..a15b482e80e130714a47d7de7a985df502080619 100644 (file)
--- a/extra/svn/Redmine.pm
+++ b/extra/svn/Redmine.pm
@@ -148,11 +148,12 @@ sub RedmineDSN {
   my ($self, $parms, $arg) = @_;
   $self->{RedmineDSN} = $arg;
   my $query = "SELECT 
-                 hashed_password, auth_source_id 
-              FROM members, projects, users 
+                 hashed_password, auth_source_id, permissions
+              FROM members, projects, users, roles
               WHERE 
                 projects.id=members.project_id 
                 AND users.id=members.user_id 
+                AND roles.id=members.role_id
                 AND users.status=1 
                 AND login=? 
                 AND identifier=? ";
@@ -277,9 +278,11 @@ sub is_member {
   $sth->execute($redmine_user, $project_id);
 
   my $ret;
-  while (my @row = $sth->fetchrow_array) {
-      unless ($row[1]) {
-          if ($row[0] eq $pass_digest) {
+  while (my ($hashed_password, $auth_source_id, $permissions) = $sth->fetchrow_array) {
+
+      unless ($auth_source_id) {
+         my $method = $r->method;
+          if ($hashed_password eq $pass_digest && (defined $read_only_methods{$method} || $permissions =~ /:commit_access/) ) {
               $ret = 1;
               last;
           }
@@ -287,7 +290,7 @@ sub is_member {
           my $sthldap = $dbh->prepare(
               "SELECT host,port,tls,account,account_password,base_dn,attr_login from auth_sources WHERE id = ?;"
           );
-          $sthldap->execute($row[1]);
+          $sthldap->execute($auth_source_id);
           while (my @rowldap = $sthldap->fetchrow_array) {
             my $ldap = Authen::Simple::LDAP->new(
                 host    =>      ($rowldap[2] == 1 || $rowldap[2] eq "t") ? "ldaps://$rowldap[0]" : $rowldap[0],

diff --git a/lib/redmine.rb b/lib/redmine.rb
index 3ba2f2cc31219b1e41fe5afad8e58f1da47ff524..346619a88684bdf5abbdd3dd08fe2bc1f6fdf490 100644 (file)
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -88,6 +88,7 @@ Redmine::AccessControl.map do |map|
     map.permission :manage_repository, {:repositories => [:edit, :destroy]}, :require => :member
     map.permission :browse_repository, :repositories => [:show, :browse, :entry, :annotate, :changes, :diff, :stats, :graph]
     map.permission :view_changesets, :repositories => [:show, :revisions, :revision]
+    map.permission :commit_access, {}
   end
 
   map.project_module :boards do |map|

diff --git a/lib/redmine/default_data/loader.rb b/lib/redmine/default_data/loader.rb
index 11bd2a0b4086038f60a51945384bfe8d0f2a600c..dd3b9e7ecadc69ae15ba549a3bf49f932c9671a7 100644 (file)
--- a/lib/redmine/default_data/loader.rb
+++ b/lib/redmine/default_data/loader.rb
@@ -67,7 +67,8 @@ module Redmine
                                                       :view_files,
                                                       :manage_files,
                                                       :browse_repository,
-                                                      :view_changesets]
+                                                      :view_changesets,
+                                                      :commit_access]
             
             reporter = Role.create! :name => l(:default_role_reporter),
                                     :position => 3,

diff --git a/test/unit/role_test.rb b/test/unit/role_test.rb
index b98af2e362eaa4150bcf39bc70f6ea1a8b66099a..cab668c504f6d71d1f57a2068721e6f78e3887b0 100644 (file)
--- a/test/unit/role_test.rb
+++ b/test/unit/role_test.rb
@@ -30,4 +30,24 @@ class RoleTest < Test::Unit::TestCase
     target.reload
     assert_equal 90, target.workflows.size
   end
+
+  def test_add_permission
+    role = Role.find(1)
+    size = role.permissions.size
+    role.add_permission!("apermission", "anotherpermission")
+    role.reload
+    assert role.permissions.include?(:anotherpermission)
+    assert_equal size + 2, role.permissions.size
+  end
+
+  def test_remove_permission
+    role = Role.find(1)
+    size = role.permissions.size
+    perm = role.permissions[0..1]
+    role.remove_permission!(*perm)
+    role.reload
+    assert ! role.permissions.include?(perm[0])
+    assert_equal size - 2, role.permissions.size
+  end
+
 end