repo & project access separated. critical gitolite bugfix

Dmitriy Zaporozhets
1 parent 2ca00bdb
Showing 13 changed files with 128 additions and 70 deletions Show diff stats
app/controllers/team_members_controller.rb
app/models/key.rb
app/models/project.rb
app/models/repository.rb
app/models/users_project.rb
app/views/projects/_team.html.haml
app/views/team_members/_show.html.haml
db/migrate/20111206213842_add_advanced_rights_to_team_member.rb
db/migrate/20111206222316_migrate_to_new_rights.rb
db/schema.rb
lib/gitlabhq/gitolite.rb
public/githost_error.html
public/gitosis_error.html
@@ -25,15 +25,10 @@ class TeamMembersController &lt; 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
@@ -23,7 +23,7 @@ class Key &lt; 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 &lt; 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
 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 &lt; 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 &lt; 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 
 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
  
@@ -4,25 +4,20 @@ class UsersProject &lt; 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
 #
@@ -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(); });
 - 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
+
@@ -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
@@ -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
@@ -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 =&gt; 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
@@ -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
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>We're sorry, but we cant get access to your gitosis</title>
+  <style type="text/css">
+    body { background-color: #EAEAEA; color: #666; text-align: center; font-family: arial, sans-serif; }
+    div.dialog {
+      width: 600px;
+      padding: 0 4em;
+      margin: 4em auto 0 auto;
+    }
+    h1 { font-size: 48px; color: #444; line-height: 1.5em; }
+    h2 { font-size: 24px; color: #666; line-height: 1.5em; }
+  </style>
+</head>
+
+<body>
+  <!-- This file lives in public/500.html -->
+  <div class="dialog">
+    <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>
@@ -1,26 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-  <title>We're sorry, but we cant get access to your gitosis</title>
-  <style type="text/css">
-    body { background-color: #EAEAEA; color: #666; text-align: center; font-family: arial, sans-serif; }
-    div.dialog {
-      width: 600px;
-      padding: 0 4em;
-      margin: 4em auto 0 auto;
-    }
-    h1 { font-size: 48px; color: #444; line-height: 1.5em; }
-    h2 { font-size: 24px; color: #666; line-height: 1.5em; }
-  </style>
-</head>
-
-<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>
-  </div>
-</body>
-</html>
...	...	@@ -25,15 +25,10 @@ class TeamMembersController < ApplicationController
25	25	@team_member = project.users_projects.find(params[:id])
26	26	@team_member.update_attributes(params[:team_member])
27	27
28		- respond_to do \|format\|
29		- format.js
30		- format.html do
31		- unless @team_member.valid?
32		- flash[:alert] = "User should have at least one role"
33		- end
34		- redirect_to team_project_path(@project)
35		- end
	28	+ unless @team_member.valid?
	29	+ flash[:alert] = "User should have at least one role"
36	30	end
	31	+ redirect_to team_project_path(@project)
37	32	end
38	33
39	34	def destroy
...	...
...	...	@@ -23,7 +23,7 @@ class Key < ActiveRecord::Base
23	23	c.update_keys(identifier, key)
24	24
25	25	projects.each do \|project\|
26		- c.update_project(project.path, project.repository_writers)
	26	+ c.update_project(project.path, project)
27	27	end
28	28	end
29	29	end
...	...	@@ -33,7 +33,7 @@ class Key < ActiveRecord::Base
33	33	c.delete_key(identifier)
34	34
35	35	projects.each do \|project\|
36		- c.update_project(project.path, project.repository_writers)
	36	+ c.update_project(project.path, project)
37	37	end
38	38	end
39	39	end
...	...
1	1	require "grit"
2	2
3	3	class Project < ActiveRecord::Base
	4	+ PROJECT_N = 0
	5	+ PROJECT_R = 1
	6	+ PROJECT_RW = 2
	7	+ PROJECT_RWA = 3
	8	+
4	9	belongs_to :owner, :class_name => "User"
5	10
6	11	has_many :merge_requests, :dependent => :destroy
...	...	@@ -47,6 +52,16 @@ class Project < ActiveRecord::Base
47	52
48	53	scope :public_only, where(:private_flag => false)
49	54
	55	+
	56	+ def self.access_options
	57	+ {
	58	+ "Denied" => PROJECT_N,
	59	+ "Read" => PROJECT_R,
	60	+ "Report" => PROJECT_RW,
	61	+ "Admin" => PROJECT_RWA
	62	+ }
	63	+ end
	64	+
50	65	def repository
51	66	@repository \|\|= Repository.new(self)
52	67	end
...	...	@@ -109,21 +124,28 @@ class Project < ActiveRecord::Base
109	124	users_projects.where(:project_id => self.id, :user_id => user.id).destroy if self.id
110	125	end
111	126
112		- def writers
113		- @writers \|\|= users_projects.includes(:user).where(:write => true).map(&:user)
	127	+ def repository_readers
	128	+ keys = Key.joins({:user => :users_projects}).
	129	+ where("users_projects.project_id = ? AND users_projects.repo_access = ?", id, Repository::REPO_R)
	130	+ keys.map(&:identifier)
114	131	end
115	132
116	133	def repository_writers
117		- keys = Key.joins({:user => :users_projects}).where("users_projects.project_id = ? AND users_projects.write = ?", id, true)
	134	+ keys = Key.joins({:user => :users_projects}).
	135	+ where("users_projects.project_id = ? AND users_projects.repo_access = ?", id, Repository::REPO_RW)
118	136	keys.map(&:identifier)
119	137	end
120	138
121	139	def readers
122		- @readers \|\|= users_projects.includes(:user).where(:read => true).map(&:user)
	140	+ @readers \|\|= users_projects.includes(:user).where(:project_access => [PROJECT_R, PROJECT_RW, PROJECT_RWA]).map(&:user)
	141	+ end
	142	+
	143	+ def writers
	144	+ @writers \|\|= users_projects.includes(:user).where(:project_access => [PROJECT_RW, PROJECT_RWA]).map(&:user)
123	145	end
124	146
125	147	def admins
126		- @admins \|\|=users_projects.includes(:user).where(:admin => true).map(&:user)
	148	+ @admins \|\|= users_projects.includes(:user).where(:project_access => PROJECT_RWA).map(&:user)
127	149	end
128	150
129	151	def root_ref
...	...
1	1	require File.join(Rails.root, "lib", "gitlabhq", "git_host")
2	2
3	3	class Repository
	4	+ REPO_N = 0
	5	+ REPO_R = 1
	6	+ REPO_RW = 2
	7	+
4	8	attr_accessor :project
5	9
6	10	def self.default_ref
7	11	"master"
8	12	end
9	13
	14	+ def self.access_options
	15	+ {
	16	+ "Denied" => REPO_N,
	17	+ "Pull" => REPO_R,
	18	+ "Pull & Push" => REPO_RW
	19	+ }
	20	+ end
	21	+
10	22	def initialize(project)
11	23	@project = project
12	24	end
...	...	@@ -33,7 +45,7 @@ class Repository
33	45
34	46	def update_repository
35	47	Gitlabhq::GitHost.system.new.configure do \|c\|
36		- c.update_project(path, project.repository_writers)
	48	+ c.update_project(path, project)
37	49	end
38	50	end
39	51
...	...
...	...	@@ -4,25 +4,20 @@ class UsersProject < ActiveRecord::Base
4	4
5	5	attr_protected :project_id, :project
6	6
7		- after_commit :update_repository
	7	+ after_save :update_repository
	8	+ after_destroy :update_repository
8	9
9	10	validates_uniqueness_of :user_id, :scope => [:project_id]
10	11	validates_presence_of :user_id
11	12	validates_presence_of :project_id
12		- validate :user_has_a_role_selected
13	13
14	14	delegate :name, :email, :to => :user, :prefix => true
15	15
16	16	def update_repository
17		- Gitosis.new.configure do \|c\|
18		- c.update_project(project.path, project.repository)
	17	+ Gitlabhq::GitHost.system.new.configure do \|c\|
	18	+ c.update_project(project.path, project)
19	19	end
20	20	end
21		-
22		- def user_has_a_role_selected
23		- errors.add(:base, "Please choose at least one Role in the Access list") unless read \|\| write \|\| admin
24		- end
25		-
26	21	end
27	22	# == Schema Information
28	23	#
...	...
...	...	@@ -5,14 +5,19 @@
5	5	%table.round-borders#team-table
6	6	%thead
7	7	%th Name
8		- %th Web
9		- %th Git
10		- %th Admin
	8	+ %th Project
	9	+ %th Repository
11	10	- if can? current_user, :admin_team_member, @project
12	11	%th Actions
13	12	- @project.users_projects.each do \|up\|
14	13	= render(:partial => 'team_members/show', :locals => {:member => up})
15	14
16	15	:javascript
	16	+ $(function(){
	17	+ $('.repo-access-select, .project-access-select').live("change", function() {
	18	+ $(this.form).submit();
	19	+ });
	20	+ })
	21	+
17	22	$('.delete-team-member').live('ajax:success', function() {
18	23	$(this).closest('tr').fadeOut(); });
...	...
1	1	- user = member.user
	2	+- allow_admin = can? current_user, :admin_project, @project
2	3	%tr{:id => dom_id(member)}
3	4	%td
4	5	= 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 @@
6	7	= link_to truncate(user.name, :lenght => 24), project_team_member_path(@project, member)
7	8	%br
8	9	.cgray{:style => "padding-top:10px;"}= truncate user.email, :lenght => 24
9		- - if can? current_user, :admin_project, @project
10		- = form_for(member, :as => :team_member, :url => project_team_member_path(@project, member)) do \|f\|
11		- %td= f.check_box :read, :onclick => "$(this.form).submit();"
12		- %td= f.check_box :write, :onclick => "$(this.form).submit();"
13		- %td= f.check_box :admin, :onclick => "$(this.form).submit();"
14		- - else
15		- %td= check_box_tag "read", 1, member.read, :disabled => :disabled
16		- %td= check_box_tag "commit", 1, member.write, :disabled => :disabled
17		- %td= check_box_tag "admin", 1, member.admin, :disabled => :disabled
18		- - if can? current_user, :admin_team_member, @project
	10	+ %td
	11	+ = form_for(member, :as => :team_member, :url => project_team_member_path(@project, member)) do \|f\|
	12	+ = f.select :project_access, options_for_select(Project.access_options, member.project_access), {}, :class => "project-access-select", :disabled => !allow_admin
	13	+ %td
	14	+ = form_for(member, :as => :team_member, :url => project_team_member_path(@project, member)) do \|f\|
	15	+ = f.select :repo_access, options_for_select(Repository.access_options, member.repo_access), {}, :class => "repo-access-select", :disabled => !allow_admin
	16	+ - if allow_admin
19	17	%td
20	18	= 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
	19	+
...	...
...	...	@@ -0,0 +1,6 @@
	1	+class AddAdvancedRightsToTeamMember < ActiveRecord::Migration
	2	+ def change
	3	+ add_column :users_projects, :repo_access, :integer, :default => 0, :null => false
	4	+ add_column :users_projects, :project_access, :integer, :default => 0, :null => false
	5	+ end
	6	+end
...	...
...	...	@@ -0,0 +1,20 @@
	1	+class MigrateToNewRights < ActiveRecord::Migration
	2	+ def up
	3	+ # Repository access
	4	+ UsersProject.update_all("repo_access = 2", :write => true)
	5	+ UsersProject.update_all("repo_access = 1", :read => true, :write => false)
	6	+
	7	+ # Project access
	8	+ UsersProject.update_all("project_access = 1", :read => true, :write => false, :admin => false)
	9	+ UsersProject.update_all("project_access = 2", :read => true, :write => true, :admin => false)
	10	+ UsersProject.update_all("project_access = 3", :read => true, :write => true, :admin => true)
	11	+
	12	+ # Remove old fields
	13	+ remove_column :users_projects, :read
	14	+ remove_column :users_projects, :write
	15	+ remove_column :users_projects, :admin
	16	+ end
	17	+
	18	+ def down
	19	+ end
	20	+end
...	...
...	...	@@ -11,7 +11,7 @@
11	11	#
12	12	# It's strongly recommended to check this file into your version control system.
13	13
14		-ActiveRecord::Schema.define(:version => 20111127155345) do
	14	+ActiveRecord::Schema.define(:version => 20111206222316) do
15	15
16	16	create_table "features", :force => true do \|t\|
17	17	t.string "name"
...	...	@@ -137,11 +137,10 @@ ActiveRecord::Schema.define(:version => 20111127155345) do
137	137	create_table "users_projects", :force => true do \|t\|
138	138	t.integer "user_id", :null => false
139	139	t.integer "project_id", :null => false
140		- t.boolean "read", :default => false
141		- t.boolean "write", :default => false
142		- t.boolean "admin", :default => false
143	140	t.datetime "created_at"
144	141	t.datetime "updated_at"
	142	+ t.integer "repo_access", :default => 0, :null => false
	143	+ t.integer "project_access", :default => 0, :null => false
145	144	end
146	145
147	146	end
...	...