diff options
author | Hannes Rosenögger <123haynes@gmail.com> | 2015-07-11 19:28:43 +0000 |
---|---|---|
committer | Hannes Rosenögger <123haynes@gmail.com> | 2015-07-11 19:28:43 +0000 |
commit | f94587eccbeda3bd0092588a12ddf9a586b29dce (patch) | |
tree | 95b8c43c9144c41fb0eb8dc6ccab2ec7f6e31c6a | |
parent | dc6aa1d34a3f78a757d829eb8cd050fa381cef99 (diff) | |
parent | 96644c1fc146b55795d36cf4c03a80d2d58d112e (diff) | |
download | gitlab-ce-f94587eccbeda3bd0092588a12ddf9a586b29dce.tar.gz |
Merge branch 'improve-autocomplete-controller-for-unknowns' into 'master'
Better handle unknown projects and groups for autocomplete
While working on !963, I noticed that specifying an unknown project ID or group ID would cause an error in the controller. This MR improves on the handling of unknown projects or groups:
1. If the user is authenticated, return a blank JSON with a 404 error.
2. Otherwise, redirect the user to login so we don't leak information about whether certain projects/groups exist.
See merge request !964
-rw-r--r-- | app/controllers/autocomplete_controller.rb | 37 | ||||
-rw-r--r-- | spec/controllers/autocomplete_controller_spec.rb | 78 |
2 files changed, 86 insertions, 29 deletions
diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 8b12643bb97..52e9c58b47c 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -2,25 +2,34 @@ class AutocompleteController < ApplicationController skip_before_action :authenticate_user!, only: [:users] def users - @users = - if params[:project_id].present? - project = Project.find(params[:project_id]) + begin + @users = + if params[:project_id].present? + project = Project.find(params[:project_id]) - if can?(current_user, :read_project, project) - project.team.users - end - elsif params[:group_id] - group = Group.find(params[:group_id]) + if can?(current_user, :read_project, project) + project.team.users + end + elsif params[:group_id] + group = Group.find(params[:group_id]) - if can?(current_user, :read_group, group) - group.users + if can?(current_user, :read_group, group) + group.users + end + elsif current_user + User.all end - elsif current_user - User.all - else - User.none + rescue ActiveRecord::RecordNotFound + if current_user + return render json: {}, status: 404 end + end + + if @users.nil? && current_user.nil? + authenticate_user! + end + @users ||= User.none @users = @users.search(params[:search]) if params[:search].present? @users = @users.active @users = @users.page(params[:page]).per(PER_PAGE) diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 9be8d0333ad..1230017c270 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -9,15 +9,27 @@ describe AutocompleteController do before do sign_in(user) project.team << [user, :master] - - get(:users, project_id: project.id) end let(:body) { JSON.parse(response.body) } - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq 1 } - it { expect(body.first["username"]).to eq user.username } + describe 'GET #users with project ID' do + before do + get(:users, project_id: project.id) + end + + it { expect(body).to be_kind_of(Array) } + it { expect(body.size).to eq 1 } + it { expect(body.first["username"]).to eq user.username } + end + + describe 'GET #users with unknown project' do + before do + get(:users, project_id: 'unknown') + end + + it { expect(response.status).to eq(404) } + end end context 'group members' do @@ -26,15 +38,27 @@ describe AutocompleteController do before do sign_in(user) group.add_owner(user) - - get(:users, group_id: group.id) end let(:body) { JSON.parse(response.body) } - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq 1 } - it { expect(body.first["username"]).to eq user.username } + describe 'GET #users with group ID' do + before do + get(:users, group_id: group.id) + end + + it { expect(body).to be_kind_of(Array) } + it { expect(body.size).to eq 1 } + it { expect(body.first["username"]).to eq user.username } + end + + describe 'GET #users with unknown group ID' do + before do + get(:users, group_id: 'unknown') + end + + it { expect(response.status).to eq(404) } + end end context 'all users' do @@ -50,26 +74,50 @@ describe AutocompleteController do end context 'unauthenticated user' do - let(:project) { create(:project, :public) } + let(:public_project) { create(:project, :public) } let(:body) { JSON.parse(response.body) } describe 'GET #users with public project' do before do - project.team << [user, :guest] - get(:users, project_id: project.id) + public_project.team << [user, :guest] + get(:users, project_id: public_project.id) end it { expect(body).to be_kind_of(Array) } it { expect(body.size).to eq 1 } end + describe 'GET #users with project' do + before do + get(:users, project_id: project.id) + end + + it { expect(response.status).to eq(302) } + end + + describe 'GET #users with unknown project' do + before do + get(:users, project_id: 'unknown') + end + + it { expect(response.status).to eq(302) } + end + + describe 'GET #users with inaccessible group' do + before do + project.team << [user, :guest] + get(:users, group_id: user.namespace.id) + end + + it { expect(response.status).to eq(302) } + end + describe 'GET #users with no project' do before do get(:users) end - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq 0 } + it { expect(response.status).to eq(302) } end end end |