From cb021e58314791c1c9eb8035ed01349876d246a1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 7 Dec 2011 01:27:07 +0200 Subject: [PATCH] repo & project access separated. critical gitolite bugfix --- app/controllers/team_members_controller.rb | 11 ++----- app/models/key.rb | 4 +-- app/models/project.rb | 32 ++++++++++++++++--- app/models/repository.rb | 14 +++++++- app/models/users_project.rb | 13 +++----- app/views/projects/_team.html.haml | 11 +++++-- app/views/team_members/_show.html.haml | 19 ++++++----- ...3842_add_advanced_rights_to_team_member.rb | 6 ++++ .../20111206222316_migrate_to_new_rights.rb | 20 ++++++++++++ db/schema.rb | 7 ++-- lib/gitlabhq/gitolite.rb | 9 ++++-- ...{gitosis_error.html => githost_error.html} | 8 ++--- 12 files changed, 106 insertions(+), 48 deletions(-) create mode 100644 db/migrate/20111206213842_add_advanced_rights_to_team_member.rb create mode 100644 db/migrate/20111206222316_migrate_to_new_rights.rb rename public/{gitosis_error.html => githost_error.html} (71%) 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 similarity index 71% rename from public/gitosis_error.html rename to public/githost_error.html index be751d6e5e6..db17eae5569 100644 --- a/public/gitosis_error.html +++ b/public/githost_error.html @@ -17,10 +17,10 @@
-

Gitosis Error

-

We're sorry, but we cant get access to your gitosis.

-

1. Check 'config/gitosis.yml' for correct settings.

-

2. Be sure web server user has access to gitosis.

+

Gitolite Error

+

We're sorry, but we cant get access to your gitolite system.

+

1. Check 'config/gitlab.yml' for correct settings.

+

2. Be sure web server user has access to gitolite.