From 061472864ceaa4dc837eebcaa583f7b81d4e7e54 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Tue, 22 Aug 2017 16:10:49 +0100 Subject: Fix group and project search for anonymous users --- app/assets/javascripts/api.js | 15 ++++++++----- app/views/search/_form.html.haml | 2 +- ...roup-and-project-search-for-anonymous-users.yml | 5 +++++ doc/api/groups.md | 9 +++++--- spec/features/search_spec.rb | 26 ++++++++++++++++++++++ spec/javascripts/api_spec.js | 26 ++++++++++++++++++++-- spec/javascripts/project_title_spec.js | 6 +++-- spec/requests/api/groups_spec.rb | 21 +++++++++++++++-- 8 files changed, 95 insertions(+), 15 deletions(-) create mode 100644 changelogs/unreleased/31409-fix-group-and-project-search-for-anonymous-users.yml diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index 56f91e95bb9..3d0ba65fd36 100644 --- a/app/assets/javascripts/api.js +++ b/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)); diff --git a/app/views/search/_form.html.haml b/app/views/search/_form.html.haml index 3139be1cd37..a4a5cec1314 100644 --- a/app/views/search/_form.html.haml +++ b/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" diff --git a/changelogs/unreleased/31409-fix-group-and-project-search-for-anonymous-users.yml b/changelogs/unreleased/31409-fix-group-and-project-search-for-anonymous-users.yml new file mode 100644 index 00000000000..06e8180db64 --- /dev/null +++ b/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 diff --git a/doc/api/groups.md b/doc/api/groups.md index 2b3d8e125c8..c2daa8bc029 100644 --- a/doc/api/groups.md +++ b/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 diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index 6742d77937f..31d509455ba 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -281,4 +281,30 @@ describe "Search" do 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 diff --git a/spec/javascripts/api_spec.js b/spec/javascripts/api_spec.js index 867322ce8ae..8c68ceff914 100644 --- a/spec/javascripts/api_spec.js +++ b/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', () => { diff --git a/spec/javascripts/project_title_spec.js b/spec/javascripts/project_title_spec.js index cc336180ff7..3d36bb3e4d4 100644 --- a/spec/javascripts/project_title_spec.js +++ b/spec/javascripts/project_title_spec.js @@ -7,6 +7,7 @@ import '~/project_select'; import '~/project'; describe('Project Title', () => { + const dummyApiVersion = 'v3000'; preloadFixtures('issues/open-issue.html.raw'); loadJSONFixtures('projects.json'); @@ -14,7 +15,7 @@ describe('Project Title', () => { 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', diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 313c38cd29c..a7557c7fb22 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -20,10 +20,15 @@ describe API::Groups do 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 @@ describe API::Groups do 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') -- cgit v1.2.1