Redirect from redirect routes to canonical routes

6 files changed, 39 insertions, 23 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