Commit 9e48f02e authored by Michael Kozono's avatar Michael Kozono

Dry up routable lookups. Fixes #30317

Note: This changes the behavior of user lookups (see the spec change) so it acts the same way as groups and projects. Unauthenticated clients attempting to access a user page will be redirected to login whether the user exists and is publicly restricted, or does not exist at all.
parent e4bcc90d
module RoutableActions module RoutableActions
extend ActiveSupport::Concern 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) def ensure_canonical_path(routable, requested_path)
return unless request.get? 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.' 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 end
end end
...@@ -9,20 +9,11 @@ class Groups::ApplicationController < ApplicationController ...@@ -9,20 +9,11 @@ class Groups::ApplicationController < ApplicationController
private private
def group def group
unless @group @group ||= find_routable!(Group, requested_full_path)
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 end
@group def requested_full_path
params[:group_id] || params[:id]
end end
def group_projects def group_projects
......
...@@ -2,6 +2,7 @@ class Projects::ApplicationController < ApplicationController ...@@ -2,6 +2,7 @@ class Projects::ApplicationController < ApplicationController
include RoutableActions include RoutableActions
skip_before_action :authenticate_user! skip_before_action :authenticate_user!
before_action :redirect_git_extension
before_action :project before_action :project
before_action :repository before_action :repository
layout 'project' layout 'project'
...@@ -10,11 +11,7 @@ class Projects::ApplicationController < ApplicationController ...@@ -10,11 +11,7 @@ class Projects::ApplicationController < ApplicationController
private private
def project def redirect_git_extension
unless @project
namespace = params[:namespace_id]
id = params[:project_id] || params[:id]
# Redirect from # Redirect from
# localhost/group/project.git # localhost/group/project.git
# to # to
...@@ -24,20 +21,20 @@ class Projects::ApplicationController < ApplicationController ...@@ -24,20 +21,20 @@ class Projects::ApplicationController < ApplicationController
redirect_to request.original_url.gsub(/\.git\/?\Z/, '') redirect_to request.original_url.gsub(/\.git\/?\Z/, '')
return return
end end
end
project_path = "#{namespace}/#{id}" def project
@project = Project.find_by_full_path(project_path, follow_redirects: request.get?) @project ||= find_routable!(Project, requested_full_path, extra_authorization_method: :project_not_being_deleted?)
if can?(current_user, :read_project, @project) && !@project.pending_delete?
ensure_canonical_path(@project, project_path)
else
@project = nil
route_not_found
end end
def requested_full_path
namespace = params[:namespace_id]
id = params[:project_id] || params[:id]
"#{namespace}/#{id}"
end end
@project def project_not_being_deleted?(project)
!project.pending_delete?
end end
def repository def repository
......
...@@ -93,16 +93,7 @@ class UsersController < ApplicationController ...@@ -93,16 +93,7 @@ class UsersController < ApplicationController
private private
def user def user
return @user if @user @user ||= find_routable!(User, params[:username])
@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
end end
def contributed_projects def contributed_projects
......
...@@ -36,9 +36,9 @@ describe UsersController do ...@@ -36,9 +36,9 @@ describe UsersController do
end end
context 'when logged out' do context 'when logged out' do
it 'renders 404' do it 'redirects to login page' do
get :show, username: user.username get :show, username: user.username
expect(response).to have_http_status(404) expect(response).to redirect_to new_user_session_path
end end
end end
...@@ -88,9 +88,9 @@ describe UsersController do ...@@ -88,9 +88,9 @@ describe UsersController do
context 'when a user by that username does not exist' do context 'when a user by that username does not exist' do
context 'when logged out' 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' get :show, username: 'nonexistent'
expect(response).to have_http_status(404) expect(response).to redirect_to new_user_session_path
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment