diff options
-rw-r--r-- | app/controllers/concerns/routable_actions.rb | 28 | ||||
-rw-r--r-- | app/controllers/groups/application_controller.rb | 17 | ||||
-rw-r--r-- | app/controllers/projects/application_controller.rb | 49 | ||||
-rw-r--r-- | app/controllers/users_controller.rb | 11 | ||||
-rw-r--r-- | spec/controllers/users_controller_spec.rb | 8 |
5 files changed, 58 insertions, 55 deletions
diff --git a/app/controllers/concerns/routable_actions.rb b/app/controllers/concerns/routable_actions.rb index 1714bc25e52..ba236fa5459 100644 --- a/app/controllers/concerns/routable_actions.rb +++ b/app/controllers/concerns/routable_actions.rb @@ -1,12 +1,36 @@ module RoutableActions extend ActiveSupport::Concern + def find_routable!(routable_klass, requested_full_path, extra_authorization_method: nil) + routable = routable_klass.find_by_full_path(requested_full_path, follow_redirects: request.get?) + + if authorized?(routable_klass, routable, extra_authorization_method) + ensure_canonical_path(routable, requested_full_path) + routable + else + route_not_found + nil + end + end + + def authorized?(routable_klass, routable, extra_authorization_method) + action = :"read_#{routable_klass.to_s.underscore}" + return false unless can?(current_user, action, routable) + + if extra_authorization_method + send(extra_authorization_method, routable) + else + true + end + end + def ensure_canonical_path(routable, requested_path) return unless request.get? - if routable.full_path != requested_path + canonical_path = routable.try(:full_path) || routable.namespace.full_path + if canonical_path != requested_path flash[:notice] = 'This project has moved to this location. Please update your links and bookmarks.' - redirect_to request.original_url.sub(requested_path, routable.full_path) + redirect_to request.original_url.sub(requested_path, canonical_path) end end end diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 209d8b1a08a..2157a56dea2 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -9,20 +9,11 @@ class Groups::ApplicationController < ApplicationController private def group - unless @group - given_path = params[:group_id] || params[:id] - @group = Group.find_by_full_path(given_path, follow_redirects: request.get?) - - if @group && can?(current_user, :read_group, @group) - ensure_canonical_path(@group, given_path) - else - @group = nil - - route_not_found - end - end + @group ||= find_routable!(Group, requested_full_path) + end - @group + def requested_full_path + params[:group_id] || params[:id] end def group_projects diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index dbdf68776f1..2301e1cca77 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -2,6 +2,7 @@ class Projects::ApplicationController < ApplicationController include RoutableActions skip_before_action :authenticate_user! + before_action :redirect_git_extension before_action :project before_action :repository layout 'project' @@ -10,34 +11,30 @@ class Projects::ApplicationController < ApplicationController private - def project - unless @project - namespace = params[:namespace_id] - id = params[:project_id] || params[:id] - - # Redirect from - # localhost/group/project.git - # to - # localhost/group/project - # - if params[:format] == 'git' - redirect_to request.original_url.gsub(/\.git\/?\Z/, '') - return - end - - project_path = "#{namespace}/#{id}" - @project = Project.find_by_full_path(project_path, follow_redirects: request.get?) - - if can?(current_user, :read_project, @project) && !@project.pending_delete? - ensure_canonical_path(@project, project_path) - else - @project = nil - - route_not_found - end + def redirect_git_extension + # Redirect from + # localhost/group/project.git + # to + # localhost/group/project + # + if params[:format] == 'git' + redirect_to request.original_url.gsub(/\.git\/?\Z/, '') + return end + end + + def project + @project ||= find_routable!(Project, requested_full_path, extra_authorization_method: :project_not_being_deleted?) + end + + def requested_full_path + namespace = params[:namespace_id] + id = params[:project_id] || params[:id] + "#{namespace}/#{id}" + end - @project + def project_not_being_deleted?(project) + !project.pending_delete? end def repository diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 67783866c3f..ca89ed221c6 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -93,16 +93,7 @@ class UsersController < ApplicationController private def user - return @user if @user - - @user = User.find_by_full_path(params[:username], follow_redirects: true) - - return render_404 unless @user - return render_404 unless can?(current_user, :read_user, @user) - - ensure_canonical_path(@user.namespace, params[:username]) - - @user + @user ||= find_routable!(User, params[:username]) end def contributed_projects diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 5e8caa89cb7..9b6b9358a40 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -36,9 +36,9 @@ describe UsersController do end context 'when logged out' do - it 'renders 404' do + it 'redirects to login page' do get :show, username: user.username - expect(response).to have_http_status(404) + expect(response).to redirect_to new_user_session_path end end @@ -88,9 +88,9 @@ describe UsersController do context 'when a user by that username does not exist' do context 'when logged out' do - it 'renders 404 (does not redirect to login)' do + it 'redirects to login page' do get :show, username: 'nonexistent' - expect(response).to have_http_status(404) + expect(response).to redirect_to new_user_session_path end end |