summaryrefslogtreecommitdiff
path: root/app
diff options
context:
space:
mode:
Diffstat (limited to 'app')
-rw-r--r--app/controllers/application_controller.rb2
-rw-r--r--app/controllers/concerns/routable_actions.rb11
-rw-r--r--app/controllers/groups/application_controller.rb21
-rw-r--r--app/controllers/groups_controller.rb2
-rw-r--r--app/controllers/projects/application_controller.rb14
-rw-r--r--app/controllers/users_controller.rb12
-rw-r--r--app/models/concerns/routable.rb24
-rw-r--r--app/models/redirect_route.rb10
-rw-r--r--app/models/user.rb5
9 files changed, 74 insertions, 27 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d2c13da6917..65a1f640a76 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -58,7 +58,7 @@ class ApplicationController < ActionController::Base
if current_user
not_found
else
- redirect_to new_user_session_path
+ authenticate_user!
end
end
diff --git a/app/controllers/concerns/routable_actions.rb b/app/controllers/concerns/routable_actions.rb
new file mode 100644
index 00000000000..6f16377a156
--- /dev/null
+++ b/app/controllers/concerns/routable_actions.rb
@@ -0,0 +1,11 @@
+module RoutableActions
+ extend ActiveSupport::Concern
+
+ def ensure_canonical_path(routable, requested_path)
+ return unless request.get?
+
+ if routable.full_path != requested_path
+ redirect_to request.original_url.sub(requested_path, routable.full_path)
+ end
+ end
+end
diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb
index 29ffaeb19c1..209d8b1a08a 100644
--- a/app/controllers/groups/application_controller.rb
+++ b/app/controllers/groups/application_controller.rb
@@ -1,4 +1,6 @@
class Groups::ApplicationController < ApplicationController
+ include RoutableActions
+
layout 'group'
skip_before_action :authenticate_user!
@@ -8,18 +10,15 @@ class Groups::ApplicationController < ApplicationController
def group
unless @group
- id = params[:group_id] || params[:id]
- @group = Group.find_by_full_path(id)
- @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute
+ given_path = params[:group_id] || params[:id]
+ @group = Group.find_by_full_path(given_path, follow_redirects: request.get?)
- unless @group && can?(current_user, :read_group, @group)
+ if @group && can?(current_user, :read_group, @group)
+ ensure_canonical_path(@group, given_path)
+ else
@group = nil
- if current_user.nil?
- authenticate_user!
- else
- render_404
- end
+ route_not_found
end
end
@@ -30,6 +29,10 @@ class Groups::ApplicationController < ApplicationController
@projects ||= GroupProjectsFinder.new(group: group, current_user: current_user).execute
end
+ def group_merge_requests
+ @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute
+ end
+
def authorize_admin_group!
unless can?(current_user, :admin_group, group)
return render_404
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 593001e6396..46c3ff10694 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -12,8 +12,8 @@ class GroupsController < Groups::ApplicationController
before_action :authorize_admin_group!, only: [:edit, :update, :destroy, :projects]
before_action :authorize_create_group!, only: [:new, :create]
- # Load group projects
before_action :group_projects, only: [:projects, :activity, :issues, :merge_requests]
+ before_action :group_merge_requests, only: [:merge_requests]
before_action :event_filter, only: [:activity]
before_action :user_actions, only: [:show, :subgroups]
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 89f1128ec36..dbdf68776f1 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -1,4 +1,6 @@
class Projects::ApplicationController < ApplicationController
+ include RoutableActions
+
skip_before_action :authenticate_user!
before_action :project
before_action :repository
@@ -24,20 +26,14 @@ class Projects::ApplicationController < ApplicationController
end
project_path = "#{namespace}/#{id}"
- @project = Project.find_by_full_path(project_path)
+ @project = Project.find_by_full_path(project_path, follow_redirects: request.get?)
if can?(current_user, :read_project, @project) && !@project.pending_delete?
- if @project.path_with_namespace != project_path
- redirect_to request.original_url.gsub(project_path, @project.path_with_namespace)
- end
+ ensure_canonical_path(@project, project_path)
else
@project = nil
- if current_user.nil?
- authenticate_user!
- else
- render_404
- end
+ route_not_found
end
end
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index a452bbba422..6b6b3b20a8d 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,7 +1,9 @@
class UsersController < ApplicationController
+ include RoutableActions
+
skip_before_action :authenticate_user!
before_action :user, except: [:exists]
- before_action :authorize_read_user!, only: [:show]
+ before_action :authorize_read_user!, except: [:exists]
def show
respond_to do |format|
@@ -92,11 +94,15 @@ class UsersController < ApplicationController
private
def authorize_read_user!
- render_404 unless can?(current_user, :read_user, user)
+ if can?(current_user, :read_user, user)
+ ensure_canonical_path(user.namespace, params[:username])
+ else
+ render_404
+ end
end
def user
- @user ||= User.find_by_username!(params[:username])
+ @user ||= User.find_by_full_path(params[:username], follow_redirects: true)
end
def contributed_projects
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index b28e05d0c28..e351dbb45dd 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -5,6 +5,7 @@ module Routable
included do
has_one :route, as: :source, autosave: true, dependent: :destroy
+ has_many :redirect_routes, as: :source, autosave: true, dependent: :destroy
validates_associated :route
validates :route, presence: true
@@ -26,16 +27,31 @@ module Routable
# Klass.find_by_full_path('gitlab-org/gitlab-ce')
#
# Returns a single object, or nil.
- def find_by_full_path(path)
+ def find_by_full_path(path, follow_redirects: false)
# On MySQL we want to ensure the ORDER BY uses a case-sensitive match so
# any literal matches come first, for this we have to use "BINARY".
# Without this there's still no guarantee in what order MySQL will return
# rows.
+ #
+ # Why do we do this?
+ #
+ # Even though we have Rails validation on Route for unique paths
+ # (case-insensitive), there are old projects in our DB (and possibly
+ # clients' DBs) that have the same path with different cases.
+ # See https://gitlab.com/gitlab-org/gitlab-ce/issues/18603. Also note that
+ # our unique index is case-sensitive in Postgres.
binary = Gitlab::Database.mysql? ? 'BINARY' : ''
-
order_sql = "(CASE WHEN #{binary} routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)"
-
- where_full_path_in([path]).reorder(order_sql).take
+ found = where_full_path_in([path]).reorder(order_sql).take
+ return found if found
+
+ if follow_redirects
+ if Gitlab::Database.postgresql?
+ joins(:redirect_routes).find_by("LOWER(redirect_routes.path) = LOWER(?)", path)
+ else
+ joins(:redirect_routes).find_by(path: path)
+ end
+ end
end
# Builds a relation to find multiple objects by their full paths.
diff --git a/app/models/redirect_route.rb b/app/models/redirect_route.rb
new file mode 100644
index 00000000000..e36ca988701
--- /dev/null
+++ b/app/models/redirect_route.rb
@@ -0,0 +1,10 @@
+class RedirectRoute < ActiveRecord::Base
+ belongs_to :source, polymorphic: true
+
+ validates :source, presence: true
+
+ validates :path,
+ length: { within: 1..255 },
+ presence: true,
+ uniqueness: { case_sensitive: false }
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index 43c5fdc038d..c6354b989ad 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -333,6 +333,11 @@ class User < ActiveRecord::Base
find_by(id: Key.unscoped.select(:user_id).where(id: key_id))
end
+ def find_by_full_path(path, follow_redirects: false)
+ namespace = Namespace.find_by_full_path(path, follow_redirects: follow_redirects)
+ namespace.owner if namespace && namespace.owner
+ end
+
def reference_prefix
'@'
end
View Lorry Controller Status