Redirect from redirect routes to canonical routes

4 files changed, 341 insertions, 21 deletions

diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 1bf0533ca24..d40aae04fc3 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -106,10 +106,9 @@ describe ApplicationController do
       controller.send(:route_not_found)
     end
 
-    it 'does redirect to login page if not authenticated' do
+    it 'does redirect to login page via authenticate_user! if not authenticated' do
       allow(controller).to receive(:current_user).and_return(nil)
-      expect(controller).to receive(:redirect_to)
-      expect(controller).to receive(:new_user_session_path)
+      expect(controller).to receive(:authenticate_user!)
       controller.send(:route_not_found)
     end
   end
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index cad82a34fb0..3398878f330 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -49,6 +49,24 @@ describe GroupsController do
         expect(assigns(:issues)).to eq [issue_2, issue_1]
       end
     end
+
+    context 'when requesting the canonical path with different casing' do
+      it 'redirects to the correct casing' do
+        get :issues, id: group.to_param.upcase
+
+        expect(response).to redirect_to(issues_group_path(group.to_param))
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { group.redirect_routes.create(path: 'old-path') }
+
+      it 'redirects to the canonical path' do
+        get :issues, id: redirect_route.path
+
+        expect(response).to redirect_to(issues_group_path(group.to_param))
+      end
+    end
   end
 
   describe 'GET #merge_requests' do
@@ -74,6 +92,24 @@ describe GroupsController do
         expect(assigns(:merge_requests)).to eq [merge_request_2, merge_request_1]
       end
     end
+
+    context 'when requesting the canonical path with different casing' do
+      it 'redirects to the correct casing' do
+        get :merge_requests, id: group.to_param.upcase
+
+        expect(response).to redirect_to(merge_requests_group_path(group.to_param))
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { group.redirect_routes.create(path: 'old-path') }
+
+      it 'redirects to the canonical path' do
+        get :merge_requests, id: redirect_route.path
+
+        expect(response).to redirect_to(merge_requests_group_path(group.to_param))
+      end
+    end
   end
 
   describe 'DELETE #destroy' do
@@ -81,7 +117,7 @@ describe GroupsController do
       it 'returns 404' do
         sign_in(create(:user))
 
-        delete :destroy, id: group.path
+        delete :destroy, id: group.to_param
 
         expect(response.status).to eq(404)
       end
@@ -94,15 +130,39 @@ describe GroupsController do
 
       it 'schedules a group destroy' do
         Sidekiq::Testing.fake! do
-          expect { delete :destroy, id: group.path }.to change(GroupDestroyWorker.jobs, :size).by(1)
+          expect { delete :destroy, id: group.to_param }.to change(GroupDestroyWorker.jobs, :size).by(1)
         end
       end
 
       it 'redirects to the root path' do
-        delete :destroy, id: group.path
+        delete :destroy, id: group.to_param
 
         expect(response).to redirect_to(root_path)
       end
+
+      context 'when requesting the canonical path with different casing' do
+        it 'does not 404' do
+          delete :destroy, id: group.to_param.upcase
+
+          expect(response).to_not have_http_status(404)
+        end
+
+        it 'does not redirect to the correct casing' do
+          delete :destroy, id: group.to_param.upcase
+
+          expect(response).to_not redirect_to(group_path(group.to_param))
+        end
+      end
+
+      context 'when requesting a redirected path' do
+        let(:redirect_route) { group.redirect_routes.create(path: 'old-path') }
+
+        it 'returns not found' do
+          delete :destroy, id: redirect_route.path
+
+          expect(response).to have_http_status(404)
+        end
+      end
     end
   end
 
@@ -111,7 +171,7 @@ describe GroupsController do
       sign_in(user)
     end
 
-    it 'updates the path succesfully' do
+    it 'updates the path successfully' do
       post :update, id: group.to_param, group: { path: 'new_path' }
 
       expect(response).to have_http_status(302)
@@ -125,5 +185,29 @@ describe GroupsController do
       expect(assigns(:group).errors).not_to be_empty
       expect(assigns(:group).path).not_to eq('new_path')
     end
+
+    context 'when requesting the canonical path with different casing' do
+      it 'does not 404' do
+        post :update, id: group.to_param.upcase, group: { path: 'new_path' }
+
+        expect(response).to_not have_http_status(404)
+      end
+
+      it 'does not redirect to the correct casing' do
+        post :update, id: group.to_param.upcase, group: { path: 'new_path' }
+
+        expect(response).to_not redirect_to(group_path(group.to_param))
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { group.redirect_routes.create(path: 'old-path') }
+
+      it 'returns not found' do
+        post :update, id: redirect_route.path, group: { path: 'new_path' }
+
+        expect(response).to have_http_status(404)
+      end
+    end
   end
 end
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index eafc2154568..1b0dd7c6369 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -218,19 +218,32 @@ describe ProjectsController do
         expect(response).to redirect_to(namespace_project_path)
       end
     end
+
+    context 'when requesting a redirected path' do
+      let!(:redirect_route) { public_project.redirect_routes.create!(path: "foo/bar") }
+
+      it 'redirects to the canonical path' do
+        get :show, namespace_id: 'foo', id: 'bar'
+
+        expect(response).to redirect_to(public_project)
+      end
+    end
   end
 
   describe "#update" do
     render_views
 
     let(:admin) { create(:admin) }
+    let(:project) { create(:project, :repository) }
+    let(:new_path) { 'renamed_path' }
+    let(:project_params) { { path: new_path } }
+
+    before do
+      sign_in(admin)
+    end
 
     it "sets the repository to the right path after a rename" do
-      project = create(:project, :repository)
-      new_path = 'renamed_path'
-      project_params = { path: new_path }
       controller.instance_variable_set(:@project, project)
-      sign_in(admin)
 
       put :update,
           namespace_id: project.namespace,
@@ -241,6 +254,34 @@ describe ProjectsController do
       expect(assigns(:repository).path).to eq(project.repository.path)
       expect(response).to have_http_status(302)
     end
+
+    context 'when requesting the canonical path' do
+      it "is case-insensitive" do
+        controller.instance_variable_set(:@project, project)
+
+        put :update,
+            namespace_id: 'FOo',
+            id: 'baR',
+            project: project_params
+
+        expect(project.repository.path).to include(new_path)
+        expect(assigns(:repository).path).to eq(project.repository.path)
+        expect(response).to have_http_status(302)
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let!(:redirect_route) { project.redirect_routes.create!(path: "foo/bar") }
+
+      it 'returns not found' do
+        put :update,
+            namespace_id: 'foo',
+            id: 'bar',
+            project: project_params
+
+        expect(response).to have_http_status(404)
+      end
+    end
   end
 
   describe "#destroy" do
@@ -276,6 +317,31 @@ describe ProjectsController do
         expect(merge_request.reload.state).to eq('closed')
       end
     end
+
+    context 'when requesting the canonical path' do
+      it "is case-insensitive" do
+        controller.instance_variable_set(:@project, project)
+        sign_in(admin)
+
+        orig_id = project.id
+        delete :destroy, namespace_id: project.namespace, id: project.path.upcase
+
+        expect { Project.find(orig_id) }.to raise_error(ActiveRecord::RecordNotFound)
+        expect(response).to have_http_status(302)
+        expect(response).to redirect_to(dashboard_projects_path)
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let!(:redirect_route) { project.redirect_routes.create!(path: "foo/bar") }
+
+      it 'returns not found' do
+        sign_in(admin)
+        delete :destroy, namespace_id: 'foo', id: 'bar'
+
+        expect(response).to have_http_status(404)
+      end
+    end
   end
 
   describe 'PUT #new_issue_address' do
@@ -397,6 +463,16 @@ describe ProjectsController do
       expect(parsed_body["Tags"]).to include("v1.0.0")
       expect(parsed_body["Commits"]).to include("123456")
     end
+
+    context 'when requesting a redirected path' do
+      let!(:redirect_route) { public_project.redirect_routes.create!(path: "foo/bar") }
+
+      it 'redirects to the canonical path' do
+        get :refs, namespace_id: 'foo', id: 'bar'
+
+        expect(response).to redirect_to(refs_namespace_project_path(namespace_id: public_project.namespace, id: public_project))
+      end
+    end
   end
 
   describe 'POST #preview_markdown' do
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index bbe9aaf737f..73fd5b6f90d 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -4,15 +4,6 @@ describe UsersController do
   let(:user) { create(:user) }
 
   describe 'GET #show' do
-    it 'is case-insensitive' do
-      user = create(:user, username: 'CamelCaseUser')
-      sign_in(user)
-
-      get :show, username: user.username.downcase
-
-      expect(response).to be_success
-    end
-
     context 'with rendered views' do
       render_views
 
@@ -61,6 +52,38 @@ describe UsersController do
         end
       end
     end
+
+    context 'when requesting the canonical path' do
+      let(:user) { create(:user, username: 'CamelCaseUser') }
+
+      before { sign_in(user) }
+
+      context 'with exactly matching casing' do
+        it 'responds with success' do
+          get :show, username: user.username
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'with different casing' do
+        it 'redirects to the correct casing' do
+          get :show, username: user.username.downcase
+
+          expect(response).to redirect_to(user)
+        end
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+      it 'redirects to the canonical path' do
+        get :show, username: redirect_route.path
+
+        expect(response).to redirect_to(user)
+      end
+    end
   end
 
   describe 'GET #calendar' do
@@ -88,11 +111,43 @@ describe UsersController do
         expect(assigns(:contributions_calendar).projects.count).to eq(2)
       end
     end
+
+    context 'when requesting the canonical path' do
+      let(:user) { create(:user, username: 'CamelCaseUser') }
+
+      before { sign_in(user) }
+
+      context 'with exactly matching casing' do
+        it 'responds with success' do
+          get :calendar, username: user.username
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'with different casing' do
+        it 'redirects to the correct casing' do
+          get :calendar, username: user.username.downcase
+
+          expect(response).to redirect_to(user_calendar_path(user))
+        end
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+      it 'redirects to the canonical path' do
+        get :calendar, username: redirect_route.path
+
+        expect(response).to redirect_to(user_calendar_path(user))
+      end
+    end
   end
 
   describe 'GET #calendar_activities' do
     let!(:project) { create(:empty_project) }
-    let!(:user) { create(:user) }
+    let(:user) { create(:user) }
 
     before do
       allow_any_instance_of(User).to receive(:contributed_projects_ids).and_return([project.id])
@@ -110,6 +165,36 @@ describe UsersController do
       get :calendar_activities, username: user.username
       expect(response).to render_template('calendar_activities')
     end
+
+    context 'when requesting the canonical path' do
+      let(:user) { create(:user, username: 'CamelCaseUser') }
+
+      context 'with exactly matching casing' do
+        it 'responds with success' do
+          get :calendar_activities, username: user.username
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'with different casing' do
+        it 'redirects to the correct casing' do
+          get :calendar_activities, username: user.username.downcase
+
+          expect(response).to redirect_to(user_calendar_activities_path(user))
+        end
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+      it 'redirects to the canonical path' do
+        get :calendar_activities, username: redirect_route.path
+
+        expect(response).to redirect_to(user_calendar_activities_path(user))
+      end
+    end
   end
 
   describe 'GET #snippets' do
@@ -132,5 +217,81 @@ describe UsersController do
         expect(JSON.parse(response.body)).to have_key('html')
       end
     end
+
+    context 'when requesting the canonical path' do
+      let(:user) { create(:user, username: 'CamelCaseUser') }
+
+      context 'with exactly matching casing' do
+        it 'responds with success' do
+          get :snippets, username: user.username
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'with different casing' do
+        it 'redirects to the correct casing' do
+          get :snippets, username: user.username.downcase
+
+          expect(response).to redirect_to(user_snippets_path(user))
+        end
+      end
+    end
+
+    context 'when requesting a redirected path' do
+      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+      it 'redirects to the canonical path' do
+        get :snippets, username: redirect_route.path
+
+        expect(response).to redirect_to(user_snippets_path(user))
+      end
+    end
+  end
+
+  describe 'GET #exists' do
+    before do
+      sign_in(user)
+    end
+
+    context 'when user exists' do
+      it 'returns JSON indicating the user exists' do
+        get :exists, username: user.username
+
+        expected_json = { exists: true }.to_json
+        expect(response.body).to eq(expected_json)
+      end
+
+      context 'when the casing is different' do
+        let(:user) { create(:user, username: 'CamelCaseUser') }
+
+        it 'returns JSON indicating the user exists' do
+          get :exists, username: user.username.downcase
+
+          expected_json = { exists: true }.to_json
+          expect(response.body).to eq(expected_json)
+        end
+      end
+    end
+
+    context 'when the user does not exist' do
+      it 'returns JSON indicating the user does not exist' do
+        get :exists, username: 'foo'
+
+        expected_json = { exists: false }.to_json
+        expect(response.body).to eq(expected_json)
+      end
+
+      context 'when a user changed their username' do
+        let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
+
+        it 'returns JSON indicating a user by that username does not exist' do
+          get :exists, username: 'old-username'
+
+          expected_json = { exists: false }.to_json
+          expect(response.body).to eq(expected_json)
+        end
+      end
+    end
   end
 end