diff --git a/app/controllers/collaborators_controller.rb b/app/controllers/collaborators_controller.rb index 091d50d09..985113b02 100644 --- a/app/controllers/collaborators_controller.rb +++ b/app/controllers/collaborators_controller.rb @@ -20,11 +20,6 @@ class CollaboratorsController < ApplicationController def edit authorize! :manage_collaborators, @project - - if params[:id] - @user = User.find params[:id] - render :edit_rights and return - end end def create diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index ad4c93bd1..dde76c200 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -33,7 +33,7 @@ class ProjectsController < ApplicationController @project = Project.new params[:project] @project.owner = get_owner - if @project.save + if @project.save! flash[:notice] = t('flash.project.saved') redirect_to @project else @@ -48,6 +48,7 @@ class ProjectsController < ApplicationController flash[:notice] = t('flash.project.saved') redirect_to @project else + @project.save! flash[:error] = t('flash.project.save_error') render :action => :edit end diff --git a/app/models/ability.rb b/app/models/ability.rb index 086564dd4..8888251b8 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -39,26 +39,26 @@ class Ability # If rule has multiple conditions CanCan joins them by 'AND' sql operator can [:read, :update, :process_build, :build, :destroy], Project, :owner_type => 'User', :owner_id => user.id - #can :read, Project, :relations => {:role => 'read'} - can :read, Project, projects_in_relations_with(:role => 'read', :object_type => 'User', :object_id => user.id) do |project| + #can :read, Project, :relations => {:role => 'reader'} + can :read, Project, projects_in_relations_with(:role => 'reader', :object_type => 'User', :object_id => user.id) do |project| #The can? and cannot? call cannot be used with a raw sql 'can' definition. - project.relations.exists?(:role => 'read', :object_type => 'User', :object_id => user.id) + project.relations.exists?(:role => 'reader', :object_type => 'User', :object_id => user.id) end - #can [:update, :process_build, :build], Project, :relations => {:role => 'write'} - can [:read, :update, :process_build, :build], Project, projects_in_relations_with(:role => ['write', 'admin'], :object_type => 'User', :object_id => user.id) do |project| - project.relations.exists?(:role => ['write', 'admin'], :object_type => 'User', :object_id => user.id) + #can [:update, :process_build, :build], Project, :relations => {:role => 'writer'} + can [:read, :update, :process_build, :build], Project, projects_in_relations_with(:role => ['writer', 'admin'], :object_type => 'User', :object_id => user.id) do |project| + project.relations.exists?(:role => ['writer', 'admin'], :object_type => 'User', :object_id => user.id) end can :manage, Platform, :owner_type => 'User', :owner_id => user.id #can :read, Platform, :members => {:id => user.id} - can :read, Platform, platforms_in_relations_with(:role => 'read', :object_type => 'User', :object_id => user.id) do |platform| - platform.relations.exists?(:role => 'read', :object_type => 'User', :object_id => user.id) + can :read, Platform, platforms_in_relations_with(:role => 'reader', :object_type => 'User', :object_id => user.id) do |platform| + platform.relations.exists?(:role => 'reader', :object_type => 'User', :object_id => user.id) end can [:manage, :add_project, :remove_project, :change_visibility, :settings], Repository, :owner_type => 'User', :owner_id => user.id #can :read, Platform, :members => {:id => user.id} - can :read, Repository, repositories_in_relations_with(:role => 'read', :object_type => 'User', :object_id => user.id) do |repository| - repository.relations.exists?(:role => 'read', :object_type => 'User', :object_id => user.id) + can :read, Repository, repositories_in_relations_with(:role => 'reader', :object_type => 'User', :object_id => user.id) do |repository| + repository.relations.exists?(:role => 'reader', :object_type => 'User', :object_id => user.id) end #can :read, Repository @@ -71,25 +71,25 @@ class Ability end can [:read, :update, :process_build, :build, :destroy], Project, :owner_type => 'Group', :owner_id => user.group_ids - #can :read, Project, :relations => {:role => 'read', :object_type => 'Group', :object_id => user.group_ids} - can :read, Project, projects_in_relations_with(:role => 'read', :object_type => 'Group', :object_id => user.group_ids) do |project| - project.relations.exists?(:role => 'read', :object_type => 'Group', :object_id => user.group_ids) + #can :read, Project, :relations => {:role => 'reader', :object_type => 'Group', :object_id => user.group_ids} + can :read, Project, projects_in_relations_with(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) do |project| + project.relations.exists?(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) end - #can [:update, :process_build, :build], Project, :relations => {:role => 'write', :object_type => 'Group', :object_id => user.group_ids} - can [:read, :update, :process_build, :build], Project, projects_in_relations_with(:role => ['write', 'admin'], :object_type => 'Group', :object_id => user.group_ids) do |project| - project.relations.exists?(:role => ['write', 'admin'], :object_type => 'Group', :object_id => user.group_ids) + #can [:update, :process_build, :build], Project, :relations => {:role => 'writer', :object_type => 'Group', :object_id => user.group_ids} + can [:read, :update, :process_build, :build], Project, projects_in_relations_with(:role => ['writer', 'admin'], :object_type => 'Group', :object_id => user.group_ids) do |project| + project.relations.exists?(:role => ['writer', 'admin'], :object_type => 'Group', :object_id => user.group_ids) end can :manage, Platform, :owner_type => 'Group', :owner_id => user.group_ids #can :read, Platform, :groups => {:id => user.group_ids} - can :read, Platform, platforms_in_relations_with(:role => 'read', :object_type => 'Group', :object_id => user.group_ids) do |platform| - platform.relations.exists?(:role => 'read', :object_type => 'Group', :object_id => user.group_ids) + can :read, Platform, platforms_in_relations_with(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) do |platform| + platform.relations.exists?(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) end can [:manage, :add_project, :remove_project], Repository, :owner_type => 'Group', :owner_id => user.group_ids #can :read, Platform, :groups => {:id => user.group_ids} - can :read, Repository, repositories_in_relations_with(:role => 'read', :object_type => 'Group', :object_id => user.group_ids) do |repository| - repository.relations.exists?(:role => 'read', :object_type => 'Group', :object_id => user.group_ids) + can :read, Repository, repositories_in_relations_with(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) do |repository| + repository.relations.exists?(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) end can(:fork, Project) {|p| can? :read, p} diff --git a/app/models/relation.rb b/app/models/relation.rb index 0fa7c57a5..dd2d4a173 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -2,7 +2,7 @@ class Relation < ActiveRecord::Base belongs_to :target, :polymorphic => true belongs_to :object, :polymorphic => true - ROLES = %w[read write admin] + ROLES = %w[reader writer admin] validates :role, :inclusion => {:in => ROLES} scope :by_object, lambda {|obj| {:conditions => ['object_id = ? AND object_type = ?', obj.id, obj.class.to_s]}} diff --git a/db/schema.rb b/db/schema.rb index 1b671f31b..602bb07f5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -229,6 +229,7 @@ ActiveRecord::Schema.define(:version => 20111122232244) do t.string "object_type" t.integer "target_id" t.string "target_type" + t.integer "role_id" t.datetime "created_at" t.datetime "updated_at" t.string "role" @@ -283,16 +284,16 @@ ActiveRecord::Schema.define(:version => 20111122232244) do create_table "users", :force => true do |t| t.string "name" - t.string "email", :default => "", :null => false - t.string "encrypted_password", :limit => 128, :default => "", :null => false - t.string "password_salt", :default => "", :null => false + t.string "email", :default => "", :null => false + t.string "encrypted_password", :limit => 128, :default => "", :null => false t.string "reset_password_token" - t.string "remember_token" + t.datetime "reset_password_sent_at" t.datetime "remember_created_at" t.datetime "created_at" t.datetime "updated_at" - t.text "ssh_key" t.string "uname" + t.text "ssh_key" + t.integer "role_id" t.integer "global_role_id" t.string "role" end diff --git a/spec/controllers/collaborators_controller_spec.rb b/spec/controllers/collaborators_controller_spec.rb index 554be0713..32ef439a1 100644 --- a/spec/controllers/collaborators_controller_spec.rb +++ b/spec/controllers/collaborators_controller_spec.rb @@ -1,5 +1,108 @@ require 'spec_helper' describe CollaboratorsController do + before(:each) do + @project = Factory(:project) + @another_user = Factory(:user) + @update_params = {:read => {@another_user.id => '1'}} + end + context 'for guest' do + it 'should not be able to perform index action' do + get :index, :project_id => @project.id + response.should redirect_to(forbidden_path) + end + + it 'should not be able to perform update action' do + get :update, {:project_id => @project.id}.merge(@update_params) + response.should redirect_to(forbidden_path) + end + end + + context 'for admin' do + before(:each) do + @admin = Factory(:admin) + set_session_for(@admin) + end + + it 'should be able to perform index action' do + get :index, :project_id => @project.id + response.should redirect_to(edit_project_collaborators_path(@project)) + end + + it 'should be able to perform update action' do + get :update, {:project_id => @project.id}.merge(@update_params) + response.should redirect_to(project_path(@project)) + end + + it 'should set flash notice on update success' do + get :update, {:project_id => @project.id}.merge(@update_params) + flash[:notice].should_not be_blank + end + end + + context 'for owner user' do + before(:each) do + @user = Factory(:user) + set_session_for(@user) + @project.update_attribute(:owner, @user) + r = @project.relations.build(:object_type => 'User', :object_id => @user.id, :role => 'admin') + r.save! + end + + it 'should be able to perform index action' do + get :index, :project_id => @project.id + response.should redirect_to(edit_project_collaborators_path(@project)) + end + + it 'should be able to perform update action' do + get :update, {:project_id => @project.id}.merge(@update_params) + response.should redirect_to(project_path(@project)) + end + + it 'should set flash notice on update success' do + get :update, {:project_id => @project.id}.merge(@update_params) + flash[:notice].should_not be_blank + end + end + + context 'for reader user' do + before(:each) do + @user = Factory(:user) + set_session_for(@user) + @project.update_attribute(:owner, @user) + r = @project.relations.build(:object_type => 'User', :object_id => @user.id, :role => 'reader') + r.save! + end + + it 'should not be able to perform index action' do + get :index, :project_id => @project.id + response.should redirect_to(forbidden_path) + end + + it 'should not be able to perform update action' do + get :update, {:project_id => @project.id}.merge(@update_params) + response.should redirect_to(forbidden_path) + end + end + + context 'for writer user' do + before(:each) do + @user = Factory(:user) + set_session_for(@user) + @project.update_attribute(:owner, @user) + r = @project.relations.build(:object_type => 'User', :object_id => @user.id, :role => 'writer') + r.save! + end + + it 'should not be able to perform index action' do + get :index, :project_id => @project.id + response.should redirect_to(forbidden_path) + end + + it 'should not be able to perform update action' do + get :update, {:project_id => @project.id}.merge(@update_params) + response.should redirect_to(forbidden_path) + end + end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 6c7a6268a..02bfcb7ba 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -1,5 +1,124 @@ require 'spec_helper' describe ProjectsController do + before(:each) do + @project = Factory(:project) + @another_user = Factory(:user) + @create_params = {:project => {:name => 'pro', :unixname => 'pro2'}} + @update_params = {:project => {:name => 'pro2', :unixname => 'pro2'}} + end + context 'for guest' do + it 'should not be able to perform index action' do + get :index, :id => @project.id + response.should redirect_to(forbidden_path) + end + + it 'should not be able to perform update action' do + get :update, {:id => @project.id}.merge(@update_params) + response.should redirect_to(forbidden_path) + end + end + + context 'for admin' do + before(:each) do + @admin = Factory(:admin) + set_session_for(@admin) + end + + it 'should be able to perform index action' do + get :index, :id => @project.id + response.should render_template(:index) + end + + it 'should be able to perform update action' do + put :update, {:id => @project.id}.merge(@update_params) + response.should redirect_to(project_path(@project)) + end + + it 'should set flash notice on update success' do + put :update, {:id => @project.id}.merge(@update_params) + flash[:notice].should_not be_blank + end + + it 'should be able to perform create action' do + post :create, @create_params + response.should redirect_to(project_path( Project.last.id )) + end + + it 'should set flash notice on create success' do + post :create, @create_params + flash[:notice].should_not be_blank + end + end + + context 'for owner user' do + before(:each) do + @user = Factory(:user) + set_session_for(@user) + @project.update_attribute(:owner, @user) + r = @project.relations.build(:object_type => 'User', :object_id => @user.id, :role => 'admin') + r.save! + end + + it 'should be able to perform update action' do + put :update, {:id => @project.id}.merge(@update_params) + response.should redirect_to(project_path(@project)) + end + + it 'should set flash notice on update success' do + put :update, {:id => @project.id}.merge(@update_params) + flash[:notice].should_not be_blank + end + end + + context 'for reader user' do + before(:each) do + @user = Factory(:user) + set_session_for(@user) + #@project.update_attribute(:owner, @user) + r = @project.relations.build(:object_type => 'User', :object_id => @user.id, :role => 'reader') + r.save! + end + + it 'should not be able to perform index action' do + get :index + response.should render_template(:index) + end + + it 'should not be able to perform show action' do + get :show, :id => @project.id + response.should render_template(:show) + end + + #it 'should not be able to perform update action' do + # get :update, {:id => @project.id}.merge(@update_params) + # response.should redirect_to(forbidden_path) + #end +# + #it 'should set flash notice on update success' do + # put :update, {:id => @project.id}.merge(@update_params) + # flash[:notice].should_not be_blank + #end + end + + context 'for writer user' do + before(:each) do + @user = Factory(:user) + set_session_for(@user) + #@project.update_attribute(:owner, @user) + r = @project.relations.build(:object_type => 'User', :object_id => @user.id, :role => 'writer') + r.save! + end + + it 'should not be able to perform update action' do + put :update, {:id => @project.id}.merge(@update_params) + response.should redirect_to(project_path(@project)) + end + + it 'should set flash notice on update success' do + put :update, {:id => @project.id}.merge(@update_params) + flash[:notice].should_not be_blank + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9b8b02c8f..d01766f68 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -24,4 +24,12 @@ RSpec.configure do |config| # examples within a transaction, remove the following line or assign false # instead of true. config.use_transactional_fixtures = true + + include Devise::TestHelpers end + +def set_session_for(user=nil) + current_user = user.is_a?(Symbol) ? Factory.create(user) : user + @request.env["devise.mapping"] = :user + sign_in current_user +end \ No newline at end of file