Fix group and project search for anonymous users

app/assets/javascripts/api.js

@@ -55,13 +55,18 @@ const Api = {
   // Return projects list. Filtered by query
   projects(query, options, callback) {
     const url = Api.buildUrl(Api.projectsPath);
+    const defaults = {
+      search: query,
+      per_page: 20,
+    };
+
+    if (gon.current_user_id) {
+      defaults.membership = true;
+    }
+
     return $.ajax({
       url,
-      data: Object.assign({
-        search: query,
-        per_page: 20,
-        membership: true,
-      }, options),
+      data: Object.assign(defaults, options),
       dataType: 'json',
     })
       .done(projects => callback(projects));

app/views/search/_form.html.haml

@@ -11,5 +11,5 @@
         %span.sr-only
           Clear search
     - unless params[:snippets].eql? 'true'
-      = render 'filter' if current_user
+      = render 'filter'
     = button_tag "Search", class: "btn btn-success btn-search"

changelogs/unreleased/31409-fix-group-and-project-search-for-anonymous-users.yml

@@ -0,0 +1,5 @@
+---
+title: Fix group and project search for anonymous users
+merge_request: 13745
+author:
+type: fixed

doc/api/groups.md

@@ -2,7 +2,8 @@
 
 ## List groups
 
-Get a list of groups. (As user: my groups or all available, as admin: all groups).
+Get a list of visible groups for the authenticated user. When accessed without
+authentication, only public groups are returned.
 
 Parameters:
 
@@ -43,7 +44,8 @@ You can search for groups by name or path, see below.
 
 ## List a group's projects
 
-Get a list of projects in this group.
+Get a list of projects in this group. When accessed without authentication, only
+public projects are returned.
 
 ```
 GET /groups/:id/projects
@@ -109,7 +111,8 @@ Example response:
 
 ## Details of a group
 
-Get all details of a group.
+Get all details of a group. This endpoint can be accessed without authentication
+if the group is publicly accessible.
 
 ```
 GET /groups/:id

spec/features/search_spec.rb

@@ -281,4 +281,30 @@
       expect(page).to have_selector('.commit-row-description', count: 9)
     end
   end
+
+  context 'anonymous user' do
+    let(:project) { create(:project, :public) }
+
+    before do
+      sign_out(user)
+    end
+
+    it 'preserves the group being searched in' do
+      visit search_path(group_id: project.namespace.id)
+
+      fill_in 'search', with: 'foo'
+      click_button 'Search'
+
+      expect(find('#group_id').value).to eq(project.namespace.id.to_s)
+    end
+
+    it 'preserves the project being searched in' do
+      visit search_path(project_id: project.id)
+
+      fill_in 'search', with: 'foo'
+      click_button 'Search'
+
+      expect(find('#project_id').value).to eq(project.id.to_s)
+    end
+  end
 end

spec/javascripts/api_spec.js

@@ -17,7 +17,7 @@ describe('Api', () => {
 
   beforeEach(() => {
     originalGon = window.gon;
-    window.gon = dummyGon;
+    window.gon = Object.assign({}, dummyGon);
   });
 
   afterEach(() => {
@@ -98,10 +98,11 @@ describe('Api', () => {
   });
 
   describe('projects', () => {
-    it('fetches projects', (done) => {
+    it('fetches projects with membership when logged in', (done) => {
       const query = 'dummy query';
       const options = { unused: 'option' };
       const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects.json?simple=true`;
+      window.gon.current_user_id = 1;
       const expectedData = Object.assign({
         search: query,
         per_page: 20,
@@ -119,6 +120,27 @@ describe('Api', () => {
         done();
       });
     });
+
+    it('fetches projects without membership when not logged in', (done) => {
+      const query = 'dummy query';
+      const options = { unused: 'option' };
+      const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects.json?simple=true`;
+      const expectedData = Object.assign({
+        search: query,
+        per_page: 20,
+      }, options);
+      spyOn(jQuery, 'ajax').and.callFake((request) => {
+        expect(request.url).toEqual(expectedUrl);
+        expect(request.dataType).toEqual('json');
+        expect(request.data).toEqual(expectedData);
+        return sendDummyResponse();
+      });
+
+      Api.projects(query, options, (response) => {
+        expect(response).toBe(dummyResponse);
+        done();
+      });
+    });
   });
 
   describe('newLabel', () => {

spec/javascripts/project_title_spec.js

@@ -7,14 +7,15 @@ import '~/project_select';
 import '~/project';
 
 describe('Project Title', () => {
+  const dummyApiVersion = 'v3000';
   preloadFixtures('issues/open-issue.html.raw');
   loadJSONFixtures('projects.json');
 
   beforeEach(() => {
     loadFixtures('issues/open-issue.html.raw');
 
     window.gon = {};
-    window.gon.api_version = 'v3';
+    window.gon.api_version = dummyApiVersion;
 
     // eslint-disable-next-line no-new
     new Project();
@@ -37,9 +38,10 @@ describe('Project Title', () => {
 
     it('toggles dropdown', () => {
       const $menu = $('.js-dropdown-menu-projects');
+      window.gon.current_user_id = 1;
       $('.js-projects-dropdown-toggle').click();
       expect($menu).toHaveClass('open');
-      expect(reqUrl).toBe('/api/v3/projects.json?simple=true');
+      expect(reqUrl).toBe(`/api/${dummyApiVersion}/projects.json?simple=true`);
       expect(reqData).toEqual({
         search: '',
         order_by: 'last_activity_at',

spec/requests/api/groups_spec.rb

@@ -20,10 +20,15 @@
 
   describe "GET /groups" do
     context "when unauthenticated" do
-      it "returns authentication error" do
+      it "returns public groups" do
         get api("/groups")
 
-        expect(response).to have_http_status(401)
+        expect(response).to have_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response)
+          .to satisfy_one { |group| group['name'] == group1.name }
       end
     end
 
@@ -165,6 +170,18 @@
   end
 
   describe "GET /groups/:id" do
+    context 'when unauthenticated' do
+      it 'returns 404 for a private group' do
+        get api("/groups/#{group2.id}")
+        expect(response).to have_http_status(404)
+      end
+
+      it 'returns 200 for a public group' do
+        get api("/groups/#{group1.id}")
+        expect(response).to have_http_status(200)
+      end
+    end
+
     context "when authenticated as user" do
       it "returns one of user1's groups" do
         project = create(:project, namespace: group2, path: 'Foo')