diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 90eddd7b297..65f346d5fe3 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -20,13 +20,13 @@ class ProjectsController < ApplicationController menu_item :roadmap, :only => :roadmap menu_item :settings, :only => :settings - before_filter :find_project, :except => [ :index, :list, :add, :copy ] - before_filter :authorize, :except => [ :index, :list, :add, :copy, :archive, :unarchive, :destroy] - before_filter :authorize_global, :only => :add + before_filter :find_project, :except => [ :index, :list, :add, :create, :copy ] + before_filter :authorize, :except => [ :index, :list, :add, :create, :copy, :archive, :unarchive, :destroy] + before_filter :authorize_global, :only => [:add, :create] before_filter :require_admin, :only => [ :copy, :archive, :unarchive, :destroy ] accept_key_auth :index - after_filter :only => [:add, :edit, :archive, :unarchive, :destroy] do |controller| + after_filter :only => [:create, :edit, :archive, :unarchive, :destroy] do |controller| if controller.request.post? controller.send :expire_action, :controller => 'welcome', :action => 'robots.txt' end @@ -65,35 +65,41 @@ def add @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position") @trackers = Tracker.all @project = Project.new(params[:project]) - if request.get? - @project.identifier = Project.next_identifier if Setting.sequential_project_identifiers? - @project.trackers = Tracker.all - @project.is_public = Setting.default_projects_public? - @project.enabled_module_names = Setting.default_projects_modules + + @project.identifier = Project.next_identifier if Setting.sequential_project_identifiers? + @project.trackers = Tracker.all + @project.is_public = Setting.default_projects_public? + @project.enabled_module_names = Setting.default_projects_modules + end + + def create + @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position") + @trackers = Tracker.all + @project = Project.new(params[:project]) + + @project.enabled_module_names = params[:enabled_modules] + if validate_parent_id && @project.save + @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id') + # Add current user as a project member if he is not admin + unless User.current.admin? + r = Role.givable.find_by_id(Setting.new_project_user_role_id.to_i) || Role.givable.first + m = Member.new(:user => User.current, :roles => [r]) + @project.members << m + end + respond_to do |format| + format.html { + flash[:notice] = l(:notice_successful_create) + redirect_to :controller => 'projects', :action => 'settings', :id => @project + } + format.xml { head :created, :location => url_for(:controller => 'projects', :action => 'show', :id => @project.id) } + end else - @project.enabled_module_names = params[:enabled_modules] - if validate_parent_id && @project.save - @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id') - # Add current user as a project member if he is not admin - unless User.current.admin? - r = Role.givable.find_by_id(Setting.new_project_user_role_id.to_i) || Role.givable.first - m = Member.new(:user => User.current, :roles => [r]) - @project.members << m - end - respond_to do |format| - format.html { - flash[:notice] = l(:notice_successful_create) - redirect_to :controller => 'projects', :action => 'settings', :id => @project - } - format.xml { head :created, :location => url_for(:controller => 'projects', :action => 'show', :id => @project.id) } - end - else - respond_to do |format| - format.html - format.xml { render :xml => @project.errors, :status => :unprocessable_entity } - end + respond_to do |format| + format.html { render :action => 'add' } + format.xml { render :xml => @project.errors, :status => :unprocessable_entity } end - end + end + end def copy diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0354d165d78..b854850a3b3 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -95,7 +95,9 @@ def edit if request.post? @user.admin = params[:user][:admin] if params[:user][:admin] @user.login = params[:user][:login] if params[:user][:login] - @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] unless params[:password].nil? or params[:password].empty? or @user.auth_source_id + if params[:password].present? && (@user.auth_source_id.nil? || params[:user][:auth_source_id].blank?) + @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] + end @user.group_ids = params[:user][:group_ids] if params[:user][:group_ids] @user.attributes = params[:user] # Was the account actived ? (do it before User#save clears the change) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 19dd654db9c..0fb44a22bfe 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -32,8 +32,27 @@ def authorize_for(controller, action) end # Display a link if user is authorized + # + # @param [String] name Anchor text (passed to link_to) + # @param [Hash, String] options Hash params or url for the link target (passed to link_to). + # This will checked by authorize_for to see if the user is authorized + # @param [optional, Hash] html_options Options passed to link_to + # @param [optional, Hash] parameters_for_method_reference Extra parameters for link_to def link_to_if_authorized(name, options = {}, html_options = nil, *parameters_for_method_reference) - link_to(name, options, html_options, *parameters_for_method_reference) if authorize_for(options[:controller] || params[:controller], options[:action]) + if options.is_a?(String) + begin + route = ActionController::Routing::Routes.recognize_path(options.gsub(/\?.*/,''), :method => options[:method] || :get) + link_controller = route[:controller] + link_action = route[:action] + rescue ActionController::RoutingError # Parse failed, not a route + link_controller, link_action = nil, nil + end + else + link_controller = options[:controller] || params[:controller] + link_action = options[:action] + end + + link_to(name, options, html_options, *parameters_for_method_reference) if authorize_for(link_controller, link_action) end # Display a link to remote if user is authorized diff --git a/app/helpers/journals_helper.rb b/app/helpers/journals_helper.rb index 990bfb1e574..c8d53f253be 100644 --- a/app/helpers/journals_helper.rb +++ b/app/helpers/journals_helper.rb @@ -39,11 +39,4 @@ def link_to_in_place_notes_editor(text, field_id, url, options={}) onclick = "new Ajax.Request('#{url_for(url)}', {asynchronous:true, evalScripts:true, method:'get'}); return false;" link_to text, '#', options.merge(:onclick => onclick) end - - def css_journal_classes(journal) - s = 'journal' - s << ' has-notes' unless journal.notes.blank? - s << ' has-details' unless journal.details.blank? - s - end end diff --git a/app/models/journal.rb b/app/models/journal.rb index a0e1ae8779e..3e846aeb89c 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -65,4 +65,12 @@ def project def attachments journalized.respond_to?(:attachments) ? journalized.attachments : nil end + + # Returns a string of css classes + def css_classes + s = 'journal' + s << ' has-notes' unless notes.blank? + s << ' has-details' unless details.blank? + s + end end diff --git a/app/views/issues/_history.rhtml b/app/views/issues/_history.rhtml index 4ea2dd2ac39..4851e5f228b 100644 --- a/app/views/issues/_history.rhtml +++ b/app/views/issues/_history.rhtml @@ -1,6 +1,6 @@ <% reply_links = authorize_for('issues', 'edit') -%> <% for journal in journals %> -
+

<%= avatar(journal.user, :size => "24") %> <%= content_tag('a', '', :name => "note-#{journal.indice}")%> diff --git a/app/views/issues/_relations.rhtml b/app/views/issues/_relations.rhtml index 71eaaa6737c..5b27fa6a577 100644 --- a/app/views/issues/_relations.rhtml +++ b/app/views/issues/_relations.rhtml @@ -28,6 +28,7 @@ <% remote_form_for(:relation, @relation, :url => {:controller => 'issue_relations', :action => 'new', :issue_id => @issue}, :method => :post, + :complete => "Form.Element.focus('relation_issue_to_id');", :html => {:id => 'new-relation-form', :style => (@relation ? '' : 'display: none;')}) do |f| %> <%= render :partial => 'issue_relations/form', :locals => {:f => f}%> <% end %> diff --git a/app/views/projects/add.rhtml b/app/views/projects/add.rhtml index d2ec5db9265..c8a9c7600d5 100644 --- a/app/views/projects/add.rhtml +++ b/app/views/projects/add.rhtml @@ -1,6 +1,6 @@

<%=l(:label_project_new)%>

-<% labelled_tabular_form_for :project, @project, :url => { :action => "add" } do |f| %> +<% labelled_tabular_form_for :project, @project, :url => { :action => "create" } do |f| %> <%= render :partial => 'form', :locals => { :f => f } %>
<%= l(:label_module_plural) %> diff --git a/config/routes.rb b/config/routes.rb index 5448b5f581d..8bcdd91d6a2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -195,9 +195,9 @@ end projects.with_options :conditions => {:method => :post} do |project_actions| - project_actions.connect 'projects/new', :action => 'add' - project_actions.connect 'projects', :action => 'add' - project_actions.connect 'projects.:format', :action => 'add', :format => /xml/ + project_actions.connect 'projects/new', :action => 'create' + project_actions.connect 'projects', :action => 'create' + project_actions.connect 'projects.:format', :action => 'create', :format => /xml/ project_actions.connect 'projects/:id/:action', :action => /edit|destroy|archive|unarchive/ project_actions.connect 'projects/:id/files/new', :controller => 'files', :action => 'new' project_actions.connect 'projects/:id/activities/save', :controller => 'project_enumerations', :action => 'save' diff --git a/lib/redmine.rb b/lib/redmine.rb index fc750d1106e..aa15770db91 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -46,12 +46,12 @@ Redmine::AccessControl.map do |map| map.permission :view_project, {:projects => [:show], :activities => [:index]}, :public => true map.permission :search_project, {:search => :index}, :public => true - map.permission :add_project, {:projects => :add}, :require => :loggedin + map.permission :add_project, {:projects => [:add, :create]}, :require => :loggedin map.permission :edit_project, {:projects => [:settings, :edit]}, :require => :member map.permission :select_project_modules, {:projects => :modules}, :require => :member map.permission :manage_members, {:projects => :settings, :members => [:new, :edit, :destroy, :autocomplete_for_member]}, :require => :member map.permission :manage_versions, {:projects => :settings, :versions => [:new, :edit, :close_completed, :destroy]}, :require => :member - map.permission :add_subprojects, {:projects => :add}, :require => :member + map.permission :add_subprojects, {:projects => [:add, :create]}, :require => :member map.project_module :issue_tracking do |map| # Issue categories diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index 4decb060fe2..6b8c8472865 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -98,9 +98,53 @@ def test_index_atom assert_response :success assert_template 'add' end + + end + + context "by non-admin user with add_project permission" do + setup do + Role.non_member.add_permission! :add_project + @request.session[:user_id] = 9 + end + + should "accept get" do + get :add + assert_response :success + assert_template 'add' + assert_no_tag :select, :attributes => {:name => 'project[parent_id]'} + end + end + + context "by non-admin user with add_subprojects permission" do + setup do + Role.find(1).remove_permission! :add_project + Role.find(1).add_permission! :add_subprojects + @request.session[:user_id] = 2 + end + + should "accept get" do + get :add, :parent_id => 'ecookbook' + assert_response :success + assert_template 'add' + # parent project selected + assert_tag :select, :attributes => {:name => 'project[parent_id]'}, + :child => {:tag => 'option', :attributes => {:value => '1', :selected => 'selected'}} + # no empty value + assert_no_tag :select, :attributes => {:name => 'project[parent_id]'}, + :child => {:tag => 'option', :attributes => {:value => ''}} + end + end + + end + + context "POST :create" do + context "by admin user" do + setup do + @request.session[:user_id] = 1 + end - should "accept post" do - post :add, :project => { :name => "blog", + should "create a new project" do + post :create, :project => { :name => "blog", :description => "weblog", :identifier => "blog", :is_public => 1, @@ -115,8 +159,8 @@ def test_index_atom assert_nil project.parent end - should "accept post with parent" do - post :add, :project => { :name => "blog", + should "create a new subproject" do + post :create, :project => { :name => "blog", :description => "weblog", :identifier => "blog", :is_public => 1, @@ -137,15 +181,8 @@ def test_index_atom @request.session[:user_id] = 9 end - should "accept get" do - get :add - assert_response :success - assert_template 'add' - assert_no_tag :select, :attributes => {:name => 'project[parent_id]'} - end - - should "accept post" do - post :add, :project => { :name => "blog", + should "accept create a Project" do + post :create, :project => { :name => "blog", :description => "weblog", :identifier => "blog", :is_public => 1, @@ -166,7 +203,7 @@ def test_index_atom should "fail with parent_id" do assert_no_difference 'Project.count' do - post :add, :project => { :name => "blog", + post :create, :project => { :name => "blog", :description => "weblog", :identifier => "blog", :is_public => 1, @@ -188,20 +225,8 @@ def test_index_atom @request.session[:user_id] = 2 end - should "accept get" do - get :add, :parent_id => 'ecookbook' - assert_response :success - assert_template 'add' - # parent project selected - assert_tag :select, :attributes => {:name => 'project[parent_id]'}, - :child => {:tag => 'option', :attributes => {:value => '1', :selected => 'selected'}} - # no empty value - assert_no_tag :select, :attributes => {:name => 'project[parent_id]'}, - :child => {:tag => 'option', :attributes => {:value => ''}} - end - - should "accept post with parent_id" do - post :add, :project => { :name => "blog", + should "create a project with a parent_id" do + post :create, :project => { :name => "blog", :description => "weblog", :identifier => "blog", :is_public => 1, @@ -214,7 +239,7 @@ def test_index_atom should "fail without parent_id" do assert_no_difference 'Project.count' do - post :add, :project => { :name => "blog", + post :create, :project => { :name => "blog", :description => "weblog", :identifier => "blog", :is_public => 1, @@ -230,7 +255,7 @@ def test_index_atom should "fail with unauthorized parent_id" do assert !User.find(2).member_of?(Project.find(6)) assert_no_difference 'Project.count' do - post :add, :project => { :name => "blog", + post :create, :project => { :name => "blog", :description => "weblog", :identifier => "blog", :is_public => 1, diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 640ce868568..0e4c14c79ac 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -143,6 +143,18 @@ def test_edit_with_password_change_should_send_a_notification assert_equal [u.mail], mail.bcc assert mail.body.include?('newpass') end + + test "POST :edit with a password change to an AuthSource user switching to Internal authentication" do + # Configure as auth source + u = User.find(2) + u.auth_source = AuthSource.find(1) + u.save! + + post :edit, :id => u.id, :user => {:auth_source_id => ''}, :password => 'newpass', :password_confirmation => 'newpass' + + assert_equal nil, u.reload.auth_source + assert_equal User.hash_password('newpass'), u.reload.hashed_password + end def test_edit_membership post :edit_membership, :id => 2, :membership_id => 1, diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 71be6c3c719..e75cf472136 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -178,8 +178,8 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/projects/33/activity", :controller => 'activities', :action => 'index', :id => '33' should_route :get, "/projects/33/activity.atom", :controller => 'activities', :action => 'index', :id => '33', :format => 'atom' - should_route :post, "/projects/new", :controller => 'projects', :action => 'add' - should_route :post, "/projects.xml", :controller => 'projects', :action => 'add', :format => 'xml' + should_route :post, "/projects/new", :controller => 'projects', :action => 'create' + should_route :post, "/projects.xml", :controller => 'projects', :action => 'create', :format => 'xml' should_route :post, "/projects/4223/edit", :controller => 'projects', :action => 'edit', :id => '4223' should_route :post, "/projects/64/destroy", :controller => 'projects', :action => 'destroy', :id => '64' should_route :post, "/projects/33/files/new", :controller => 'files', :action => 'new', :id => '33' diff --git a/test/unit/helpers/application_helper_test.rb b/test/unit/helpers/application_helper_test.rb index 6fd21fe376a..1936a981b86 100644 --- a/test/unit/helpers/application_helper_test.rb +++ b/test/unit/helpers/application_helper_test.rb @@ -17,10 +17,7 @@ require File.dirname(__FILE__) + '/../../test_helper' -class ApplicationHelperTest < HelperTestCase - include ApplicationHelper - include ActionView::Helpers::TextHelper - include ActionView::Helpers::DateHelper +class ApplicationHelperTest < ActionView::TestCase fixtures :projects, :roles, :enabled_modules, :users, :repositories, :changesets, @@ -33,6 +30,35 @@ class ApplicationHelperTest < HelperTestCase def setup super end + + context "#link_to_if_authorized" do + context "authorized user" do + should "be tested" + end + + context "unauthorized user" do + should "be tested" + end + + should "allow using the :controller and :action for the target link" do + User.current = User.find_by_login('admin') + + @project = Issue.first.project # Used by helper + response = link_to_if_authorized("By controller/action", + {:controller => 'issues', :action => 'edit', :id => Issue.first.id}) + assert_match /href/, response + end + + should "allow using the url for the target link" do + User.current = User.find_by_login('admin') + + @project = Issue.first.project # Used by helper + response = link_to_if_authorized("By url", + new_issue_move_path(:id => Issue.first.id)) + assert_match /href/, response + end + + end def test_auto_links to_test = {