adds fix for security issue when annonymous user does not have access to repository we now display the activity feed instead of the readme23990-project-show-error-when-empty-repo

5 files changed, 92 insertions, 70 deletions

diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb
index f7189e0c5a1..6e68aad4cb7 100644
--- a/app/helpers/preferences_helper.rb
+++ b/app/helpers/preferences_helper.rb
@@ -50,7 +50,7 @@ module PreferencesHelper
   end
 
   def default_project_view
-    return annonymous_project_view unless current_user
+    return anonymous_project_view unless current_user
 
     user_view = current_user.project_view
 
@@ -67,7 +67,7 @@ module PreferencesHelper
     end
   end
 
-  def annonymous_project_view
-    @project.empty_repo? ? 'empty' : 'readme'
+  def anonymous_project_view
+    @project.empty_repo? || !can?(current_user, :download_code, @project) ? 'activity' : 'readme'
   end
 end
diff --git a/app/views/projects/_empty.html.haml b/app/views/projects/_empty.html.haml
deleted file mode 100644
index 56276e164de..00000000000
--- a/app/views/projects/_empty.html.haml
+++ /dev/null
@@ -1,58 +0,0 @@
-.row-content-block.second-block.center
-  %h3.page-title
-    The repository for this project is empty
-  - if can?(current_user, :push_code, @project)
-    %p
-      If you already have files you can push them using command line instructions below.
-    %p
-      Otherwise you can start with adding a
-      = succeed ',' do
-        = link_to "README", new_readme_path, class: 'underlined-link'
-      a
-      = succeed ',' do
-        = link_to "LICENSE", add_special_file_path(@project, file_name: 'LICENSE'), class: 'underlined-link'
-      or a
-      = link_to '.gitignore', add_special_file_path(@project, file_name: '.gitignore'), class: 'underlined-link'
-      to this project.
-    %p
-      You will need to be owner or have the master permission level for the initial push, as the master branch is automatically protected.
-
-- if can?(current_user, :push_code, @project)
-  %div{ class: container_class }
-    .prepend-top-20
-    .empty_wrapper
-      %h3.page-title-empty
-        Command line instructions
-      %div.git-empty
-        %fieldset
-          %h5 Git global setup
-          %pre.light-well
-            :preserve
-              git config --global user.name "#{h git_user_name}"
-              git config --global user.email "#{h git_user_email}"
-
-        %fieldset
-          %h5 Create a new repository
-          %pre.light-well
-            :preserve
-              git clone #{ content_tag(:span, default_url_to_repo, class: 'clone')}
-              cd #{h @project.path}
-              touch README.md
-              git add README.md
-              git commit -m "add README"
-              git push -u origin master
-
-        %fieldset
-          %h5 Existing folder or Git repository
-          %pre.light-well
-            :preserve
-              cd existing_folder
-              git init
-              git remote add origin #{ content_tag(:span, default_url_to_repo, class: 'clone')}
-              git add .
-              git commit
-              git push -u origin master
-
-          - if can? current_user, :remove_project, @project
-            .prepend-top-20
-              = link_to 'Remove project', [@project.namespace.becomes(Namespace), @project], data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn btn-remove pull-right"
diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml
index 94895699453..7a39064adc5 100644
--- a/app/views/projects/empty.html.haml
+++ b/app/views/projects/empty.html.haml
@@ -6,4 +6,62 @@
     = render 'shared/no_password'
 
 = render "home_panel"
-= render "empty"
+
+.row-content-block.second-block.center
+  %h3.page-title
+    The repository for this project is empty
+  - if can?(current_user, :push_code, @project)
+    %p
+      If you already have files you can push them using command line instructions below.
+    %p
+      Otherwise you can start with adding a
+      = succeed ',' do
+        = link_to "README", new_readme_path, class: 'underlined-link'
+      a
+      = succeed ',' do
+        = link_to "LICENSE", add_special_file_path(@project, file_name: 'LICENSE'), class: 'underlined-link'
+      or a
+      = link_to '.gitignore', add_special_file_path(@project, file_name: '.gitignore'), class: 'underlined-link'
+      to this project.
+    %p
+      You will need to be owner or have the master permission level for the initial push, as the master branch is automatically protected.
+
+- if can?(current_user, :push_code, @project)
+  %div{ class: container_class }
+    .prepend-top-20
+    .empty_wrapper
+      %h3.page-title-empty
+        Command line instructions
+      %div.git-empty
+        %fieldset
+          %h5 Git global setup
+          %pre.light-well
+            :preserve
+              git config --global user.name "#{h git_user_name}"
+              git config --global user.email "#{h git_user_email}"
+
+        %fieldset
+          %h5 Create a new repository
+          %pre.light-well
+            :preserve
+              git clone #{ content_tag(:span, default_url_to_repo, class: 'clone')}
+              cd #{h @project.path}
+              touch README.md
+              git add README.md
+              git commit -m "add README"
+              git push -u origin master
+
+        %fieldset
+          %h5 Existing folder or Git repository
+          %pre.light-well
+            :preserve
+              cd existing_folder
+              git init
+              git remote add origin #{ content_tag(:span, default_url_to_repo, class: 'clone')}
+              git add .
+              git commit
+              git push -u origin master
+
+          - if can? current_user, :remove_project, @project
+            .prepend-top-20
+              = link_to 'Remove project', [@project.namespace.becomes(Namespace), @project], data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn btn-remove pull-right"
diff --git a/changelogs/unreleased/23990-project-show-error-when-empty-repo.yml b/changelogs/unreleased/23990-project-show-error-when-empty-repo.yml
index 040737f917c..8d4593d4df7 100644
--- a/changelogs/unreleased/23990-project-show-error-when-empty-repo.yml
+++ b/changelogs/unreleased/23990-project-show-error-when-empty-repo.yml
@@ -1,4 +1,4 @@
 ---
-title: 500 error on project show when user is not logged in and project is still empty
+title: fixes 500 error on project show when user is not logged in and project is still empty
 merge_request: 7376
 author: 
diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb
index 02b464f7e07..77841e85223 100644
--- a/spec/helpers/preferences_helper_spec.rb
+++ b/spec/helpers/preferences_helper_spec.rb
@@ -86,21 +86,43 @@ describe PreferencesHelper do
     end
   end
 
-  describe 'default_project_view' do
+  describe '#default_project_view' do
     context 'user not signed in' do
       before do
-        @project = create(:project)
+        helper.instance_variable_set(:@project, project)
         stub_user
       end
 
-      it 'returns readme view if repository is not empty' do
-        expect(helper.default_project_view).to eq('readme')
+      context 'when repository is empty' do
+        let(:project) { create(:project_empty_repo, :public) }
+
+        it 'returns activity if user has repository access' do
+          allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(true)
+
+          expect(helper.default_project_view).to eq('activity')
+        end
+
+        it 'returns activity if user does not have repository access' do
+          allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(false)
+
+          expect(helper.default_project_view).to eq('activity')
+        end
       end
 
-      it 'returns activity if repository is empty' do
-        expect(@project).to receive(:empty_repo?).and_return(true)
+      context 'when repository is not empty' do
+        let(:project) { create(:project, :public) }
+
+        it 'returns readme if user has repository access' do
+          allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(true)
+
+          expect(helper.default_project_view).to eq('readme')
+        end
+
+        it 'returns activity if user does not have repository access' do
+          allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(false)
 
-        expect(helper.default_project_view).to eq('empty')
+          expect(helper.default_project_view).to eq('activity')
+        end
       end
     end
   end