@@ -13,19 +13,17 @@ class ProjectsController < ApplicationController format.xml { head :found, :location => project_path(@projects.first, :format => 'xml') } else format.html - format.rss { render_index_rss } + format.rss { index_rss } format.xml { render :xml => @projects } end end end def show - @project = @projects.find! params[:id] - respond_to do |format| - format.html { redirect_to @project.path_to_first_menu_item } - format.rss { render_show_rss(@project) } - format.xml { render :xml => @project } + format.html { show_html } + format.rss { find_project!; show_rss(@project) } + format.xml { find_project!; render :xml => @project } end end @@ -38,6 +36,10 @@ class ProjectsController < ApplicationController end end + def find_project! + @project = @projects.find! params[:id] + end + def project_has_no_accessible_menu_items?(project) project.enabled_menu_items.find do |item| path = item.path(self, project) @@ -51,7 +53,7 @@ class ProjectsController < ApplicationController end.nil? end - def render_index_rss + def index_rss @records = User.current.projects.active.inject([]) do |result, project| find_feedable_records(project).each do |record| result << [record, project] @@ -68,7 +70,22 @@ class ProjectsController < ApplicationController end end - def render_show_rss(project) + def show_html + @project = @projects.find params[:id] + + if @project + redirect_to @project.path_to_first_menu_item + elsif @projects.any? + redirect_to(projects_path) + else + # Fail with 404 if the project genuinely doesn't exist + Project.active.find params[:id] + # Fail authorization otherwise + failed_authorization! + end + end + + def show_rss(project) render_rss project.name, _('All news for {{project}}', :project => project.name), project_url(project) do |items| app/controllers/projects_controller.rb @@ -183,8 +183,8 @@ module Retrospectiva def failed_authorization! project = Project.current ? Project.current.name : 'nil' user = User.current ? User.current.name : 'nil' - permissions = self.class.require_permissions[action_name] - + permissions = self.class.authorized_controller? ? self.class.require_permissions[action_name] : nil + raise Retrospectiva::AccessManager::NoAuthorizationError, "No authorization for #{self.class.name}/#{action_name} - params: #{params.except(:controller, :action).inspect}, user: #{user}, project: #{project}, permissions: #{permissions.inspect}" end lib/retrospectiva/access_manager/secure_controller.rb @@ -90,7 +90,7 @@ describe 'real-world behaviour' do before do @user = mock_current_user! :name => 'Public', :public? => true, :admin? => false, :permitted? => false - @project = mock_model Project, + @project = stub_model Project, :name => 'Retro', :short_name => 'retro', :enabled_modules => ['milestones'] spec/controllers/project_area_controller_spec.rb @@ -3,7 +3,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe ProjectsController do before do - @user = mock_current_user! + @user = mock_current_user! :name => 'Someone' end describe 'loading active projects' do @@ -237,10 +237,9 @@ describe ProjectsController do controller.stub!(:find_feedable_records).and_return([@changeset, @ticket]) controller.stub!(:project_has_no_accessible_menu_items?).and_return(false) - @projects = [mock_model(Project, :to_param => 'retro', :name => 'Retro', :path_to_first_menu_item => '/projects/retro/changesets')] - @projects.stub!(:active).and_return(@projects) - @user.should_receive(:projects).and_return(@projects) - @projects.stub!(:find!).and_return(@projects.first) + @project = mock_model Project, :to_param => 'retro', :name => 'Retro', :path_to_first_menu_item => '/projects/retro/changesets', :active? => true + @projects = AssociationProxies::UserProjects.new([@project]) + @user.stub!(:projects).and_return(@projects) end def do_get(project_name, format = 'html') @@ -248,20 +247,53 @@ describe ProjectsController do end describe 'for HTML requests' do - + it 'should redirect to the first available menu item' do - @projects.should_receive(:find!).with('retro').and_return(@projects.first) do_get 'retro' response.should be_redirect - response.should redirect_to(project_changesets_path(@projects.first)) + response.should redirect_to(project_changesets_path(@project)) end + describe 'if project is not accessible and other projects are present' do + + it 'should redirect to the projects overview page' do + do_get 'other' + response.should be_redirect + response.should redirect_to(projects_path) + end + + end + + describe 'if project is not accessible and no other projects are present' do + before do + @projects = AssociationProxies::UserProjects.new + @user.stub!(:projects).and_return(@projects) + end + + describe 'but project is does actually exist (just the user has no permission to see it)' do + + it 'should fail authorization' do + Project.should_receive(:find).with('other').and_return(stub_model(Project)) + lambda { do_get 'other' }.should raise_error(Retrospectiva::AccessManager::NoAuthorizationError) + end + + end + + describe 'and project is genuinely doesn\'t exist' do + + it 'should fail with 404' do + lambda { do_get 'other' }.should raise_error(ActiveRecord::RecordNotFound) + end + + end + + end end describe 'for RSS requests' do it 'should load the feedable records' do - controller.should_receive(:find_feedable_records).with(@projects.first).and_return([@changeset, @ticket]) + controller.should_receive(:find_feedable_records).with(@project).and_return([@changeset, @ticket]) do_get 'retro', 'rss' end spec/controllers/projects_controller_spec.rb @@ -20,7 +20,8 @@ describe Notifications do :domain => mock('Setting::Domain', :name => 'domain', :value => 'mydomain.com'), :authentication => mock('Setting::AuthType', :name => 'authentication', :value => 'plain'), :user_name => mock('Setting::User', :name => 'user_name', :value => 'me'), - :password => mock('Setting::Pass', :name => 'password', :value => 'secret') + :password => mock('Setting::Pass', :name => 'password', :value => 'secret'), + :enable_starttls_auto => mock('Setting::AutoTLS', :name => 'enable_starttls_auto', :value => true) } end spec/models/notifications_spec.rb @@ -9,7 +9,7 @@ module Spec::TypicalMilestonesControllerSetup @milestones.stub!(:count) @milestones.stub!(:maximum) - @project = mock_model(Project, :name => 'Retro', :short_name => 'retro', :milestones => @milestones) + @project = stub_model(Project, :name => 'Retro', :short_name => 'retro', :milestones => @milestones) @projects = [@project] @projects.stub!(:active).and_return(@projects) @projects.stub!(:find).and_return(@project) spec/support/helpers/typical_milestones_controller_setup.rb 8f5cee4513d7f48a6151daa33363399b30178e6e Dimitrij Denissenko contact@dvisionfactory.com http://github.com/dim/retrospectiva/commit/55924800f09052516ab8f499faa3fe676d853671 55924800f09052516ab8f499faa3fe676d853671 2009-10-23T00:34:25-07:00 2009-10-23T00:34:25-07:00 Be more flexible on project selection [#692] / Fixed specs 215d648c97883bcc7c5e2282f9a113e736269e3c Dimitrij Denissenko contact@dvisionfactory.com