repo & project access separated. critical gitolite bugfix

12 files changed, 106 insertions, 48 deletions

diff --git a/app/controllers/team_members_controller.rb b/app/controllers/team_members_controller.rb
index 8a4e32e59c5..b17c9a30d88 100644
--- a/app/controllers/team_members_controller.rb
+++ b/app/controllers/team_members_controller.rb
@@ -25,15 +25,10 @@ class TeamMembersController < ApplicationController
     @team_member = project.users_projects.find(params[:id])
     @team_member.update_attributes(params[:team_member])
 
-    respond_to do |format|
-      format.js
-      format.html do
-        unless @team_member.valid?
-          flash[:alert] = "User should have at least one role"
-        end
-        redirect_to team_project_path(@project)
-      end
+    unless @team_member.valid?
+      flash[:alert] = "User should have at least one role"
     end
+    redirect_to team_project_path(@project)
   end
 
   def destroy
diff --git a/app/models/key.rb b/app/models/key.rb
index e265842caa8..83e4fc79798 100644
--- a/app/models/key.rb
+++ b/app/models/key.rb
@@ -23,7 +23,7 @@ class Key < ActiveRecord::Base
       c.update_keys(identifier, key)
 
       projects.each do |project|
-        c.update_project(project.path, project.repository_writers)
+        c.update_project(project.path, project)
       end
     end
   end
@@ -33,7 +33,7 @@ class Key < ActiveRecord::Base
       c.delete_key(identifier)
 
       projects.each do |project|
-        c.update_project(project.path, project.repository_writers)
+        c.update_project(project.path, project)
       end
     end
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index d78513d42f7..65b4af3d409 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1,6 +1,11 @@
 require "grit"
 
 class Project < ActiveRecord::Base
+  PROJECT_N = 0
+  PROJECT_R = 1
+  PROJECT_RW = 2
+  PROJECT_RWA = 3
+
   belongs_to :owner, :class_name => "User"
 
   has_many :merge_requests, :dependent => :destroy
@@ -47,6 +52,16 @@ class Project < ActiveRecord::Base
 
   scope :public_only, where(:private_flag => false)
 
+
+  def self.access_options
+    {
+      "Denied" => PROJECT_N,
+      "Read"   => PROJECT_R,
+      "Report" => PROJECT_RW,
+      "Admin"  => PROJECT_RWA
+    }
+  end
+
   def repository
     @repository ||= Repository.new(self)
   end
@@ -109,21 +124,28 @@ class Project < ActiveRecord::Base
     users_projects.where(:project_id => self.id, :user_id => user.id).destroy if self.id
   end
 
-  def writers
-    @writers ||= users_projects.includes(:user).where(:write => true).map(&:user)
+  def repository_readers
+    keys = Key.joins({:user => :users_projects}).
+      where("users_projects.project_id = ? AND users_projects.repo_access = ?", id, Repository::REPO_R)
+    keys.map(&:identifier)
   end
 
   def repository_writers
-    keys = Key.joins({:user => :users_projects}).where("users_projects.project_id = ? AND users_projects.write = ?", id, true)
+    keys = Key.joins({:user => :users_projects}).
+      where("users_projects.project_id = ? AND users_projects.repo_access = ?", id, Repository::REPO_RW)
     keys.map(&:identifier)
   end
 
   def readers
-    @readers ||= users_projects.includes(:user).where(:read => true).map(&:user)
+    @readers ||= users_projects.includes(:user).where(:project_access => [PROJECT_R, PROJECT_RW, PROJECT_RWA]).map(&:user)
+  end
+
+  def writers
+    @writers ||= users_projects.includes(:user).where(:project_access => [PROJECT_RW, PROJECT_RWA]).map(&:user)
   end
 
   def admins
-    @admins ||=users_projects.includes(:user).where(:admin => true).map(&:user)
+    @admins ||= users_projects.includes(:user).where(:project_access => PROJECT_RWA).map(&:user)
   end
 
   def root_ref 
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 7140719556e..0e6f0e9a8f9 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1,12 +1,24 @@
 require File.join(Rails.root, "lib", "gitlabhq", "git_host")
 
 class Repository
+  REPO_N = 0
+  REPO_R = 1
+  REPO_RW = 2
+
   attr_accessor :project
 
   def self.default_ref
     "master"
   end
 
+  def self.access_options
+    {
+      "Denied"      => REPO_N,
+      "Pull"        => REPO_R,
+      "Pull & Push" => REPO_RW
+    }
+  end
+
   def initialize(project)
     @project = project
   end
@@ -33,7 +45,7 @@ class Repository
 
   def update_repository
     Gitlabhq::GitHost.system.new.configure do |c|
-      c.update_project(path, project.repository_writers)
+      c.update_project(path, project)
     end
   end
 
diff --git a/app/models/users_project.rb b/app/models/users_project.rb
index 9a114087b93..84d84cf3899 100644
--- a/app/models/users_project.rb
+++ b/app/models/users_project.rb
@@ -4,25 +4,20 @@ class UsersProject < ActiveRecord::Base
 
   attr_protected :project_id, :project
 
-  after_commit :update_repository
+  after_save :update_repository
+  after_destroy :update_repository
 
   validates_uniqueness_of :user_id, :scope => [:project_id]
   validates_presence_of :user_id
   validates_presence_of :project_id
-  validate :user_has_a_role_selected
 
   delegate :name, :email, :to => :user, :prefix => true
 
   def update_repository
-    Gitosis.new.configure do |c|
-      c.update_project(project.path, project.repository)
+    Gitlabhq::GitHost.system.new.configure do |c|
+      c.update_project(project.path, project)
     end
   end
-
-  def user_has_a_role_selected
-    errors.add(:base, "Please choose at least one Role in the Access list") unless read || write || admin
-  end
-
 end
 # == Schema Information
 #
diff --git a/app/views/projects/_team.html.haml b/app/views/projects/_team.html.haml
index 79b1edbf9a8..ebf3e9a8ec0 100644
--- a/app/views/projects/_team.html.haml
+++ b/app/views/projects/_team.html.haml
@@ -5,14 +5,19 @@
 %table.round-borders#team-table
   %thead
     %th Name
-    %th Web
-    %th Git
-    %th Admin
+    %th Project
+    %th Repository
     - if can? current_user, :admin_team_member, @project
       %th Actions
   - @project.users_projects.each do |up|
     = render(:partial => 'team_members/show', :locals => {:member => up})
 
 :javascript
+  $(function(){
+    $('.repo-access-select, .project-access-select').live("change", function() { 
+      $(this.form).submit();
+    });
+  })
+
   $('.delete-team-member').live('ajax:success', function() {
     $(this).closest('tr').fadeOut(); });
diff --git a/app/views/team_members/_show.html.haml b/app/views/team_members/_show.html.haml
index 1fb3cfae1b7..b0e8f17204f 100644
--- a/app/views/team_members/_show.html.haml
+++ b/app/views/team_members/_show.html.haml
@@ -1,4 +1,5 @@
 - user = member.user
+- allow_admin = can? current_user, :admin_project, @project
 %tr{:id => dom_id(member)}
   %td
     = link_to image_tag(gravatar_icon(user.email), :class => "left", :width => 40, :style => "padding:0 5px;"), project_team_member_path(@project, member)
@@ -6,15 +7,13 @@
     = link_to truncate(user.name, :lenght => 24), project_team_member_path(@project, member)
     %br
     .cgray{:style => "padding-top:10px;"}= truncate user.email, :lenght => 24
-  - if can? current_user, :admin_project, @project
-    = form_for(member, :as => :team_member, :url => project_team_member_path(@project, member)) do |f|
-      %td= f.check_box :read, :onclick => "$(this.form).submit();"
-      %td= f.check_box :write, :onclick => "$(this.form).submit();"
-      %td= f.check_box :admin, :onclick => "$(this.form).submit();"
-  - else
-    %td= check_box_tag "read",   1, member.read, :disabled => :disabled
-    %td= check_box_tag "commit", 1, member.write, :disabled => :disabled
-    %td= check_box_tag "admin",  1, member.admin, :disabled => :disabled
-  - if can? current_user, :admin_team_member, @project
+    %td
+      = form_for(member, :as => :team_member, :url => project_team_member_path(@project, member)) do |f|
+        = f.select :project_access, options_for_select(Project.access_options, member.project_access), {}, :class => "project-access-select", :disabled => !allow_admin
+    %td
+      = form_for(member, :as => :team_member, :url => project_team_member_path(@project, member)) do |f|
+        = f.select :repo_access, options_for_select(Repository.access_options, member.repo_access), {}, :class => "repo-access-select", :disabled => !allow_admin
+  - if allow_admin
     %td
       = link_to 'Cancel', project_team_member_path(:project_id => @project, :id => member.id), :confirm => 'Are you sure?', :method => :delete, :class => "grey-button negative delete-team-member", :remote => true
+
diff --git a/db/migrate/20111206213842_add_advanced_rights_to_team_member.rb b/db/migrate/20111206213842_add_advanced_rights_to_team_member.rb
new file mode 100644
index 00000000000..e2695fdafa7
--- /dev/null
+++ b/db/migrate/20111206213842_add_advanced_rights_to_team_member.rb
@@ -0,0 +1,6 @@
+class AddAdvancedRightsToTeamMember < ActiveRecord::Migration
+  def change
+    add_column :users_projects, :repo_access, :integer, :default => 0, :null => false
+    add_column :users_projects, :project_access, :integer, :default => 0, :null => false
+  end
+end
diff --git a/db/migrate/20111206222316_migrate_to_new_rights.rb b/db/migrate/20111206222316_migrate_to_new_rights.rb
new file mode 100644
index 00000000000..22e0c1ce994
--- /dev/null
+++ b/db/migrate/20111206222316_migrate_to_new_rights.rb
@@ -0,0 +1,20 @@
+class MigrateToNewRights < ActiveRecord::Migration
+  def up
+    # Repository access
+    UsersProject.update_all("repo_access = 2", :write => true)
+    UsersProject.update_all("repo_access = 1", :read => true, :write => false)
+
+    # Project access
+    UsersProject.update_all("project_access = 1", :read => true, :write => false, :admin => false)
+    UsersProject.update_all("project_access = 2", :read => true, :write => true, :admin => false)
+    UsersProject.update_all("project_access = 3", :read => true, :write => true, :admin => true)
+
+    # Remove old fields
+    remove_column :users_projects, :read
+    remove_column :users_projects, :write
+    remove_column :users_projects, :admin
+  end
+
+  def down
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 8ffd874c7b7..fd50bf32c52 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20111127155345) do
+ActiveRecord::Schema.define(:version => 20111206222316) do
 
   create_table "features", :force => true do |t|
     t.string   "name"
@@ -137,11 +137,10 @@ ActiveRecord::Schema.define(:version => 20111127155345) do
   create_table "users_projects", :force => true do |t|
     t.integer  "user_id",                       :null => false
     t.integer  "project_id",                    :null => false
-    t.boolean  "read",       :default => false
-    t.boolean  "write",      :default => false
-    t.boolean  "admin",      :default => false
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.integer  "repo_access",    :default => 0, :null => false
+    t.integer  "project_access", :default => 0, :null => false
   end
 
 end
diff --git a/lib/gitlabhq/gitolite.rb b/lib/gitlabhq/gitolite.rb
index de8241fe13a..10ab87bdaf8 100644
--- a/lib/gitlabhq/gitolite.rb
+++ b/lib/gitlabhq/gitolite.rb
@@ -61,7 +61,7 @@ module Gitlabhq
     end
 
     # update or create
-    def update_project(repo_name, name_writers)
+    def update_project(repo_name, project)
       ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(@local_dir,'gitolite'))
       conf = ga_repo.config
 
@@ -71,8 +71,13 @@ module Gitlabhq
                ::Gitolite::Config::Repo.new(repo_name)
              end
 
+      name_readers = project.repository_readers
+      name_writers = project.repository_writers
+
+      repo.clean_permissions
+      repo.add_permission("R", "", name_readers) unless name_readers.blank?
       repo.add_permission("RW+", "", name_writers) unless name_writers.blank?
-      conf.add_repo(repo)
+      conf.add_repo(repo, true)
 
       ga_repo.save
     end
diff --git a/public/gitosis_error.html b/public/githost_error.html
index be751d6e5e6..db17eae5569 100644
--- a/public/gitosis_error.html
+++ b/public/githost_error.html
@@ -17,10 +17,10 @@
 <body>
   <!-- This file lives in public/500.html -->
   <div class="dialog">
-    <h1>Gitosis Error</h1>
-    <h2>We're sorry, but we cant get access to your gitosis.</h2>
-    <h3> 1. Check 'config/gitosis.yml' for correct settings.</h3>
-    <h3> 2. Be sure web server user has access to gitosis.</h3>
+    <h1>Gitolite Error</h1>
+    <h2>We're sorry, but we cant get access to your gitolite system.</h2>
+    <h3> 1. Check 'config/gitlab.yml' for correct settings.</h3>
+    <h3> 2. Be sure web server user has access to gitolite.</h3>
   </div>
 </body>
 </html>