Commit f94587ec authored by Hannes Rosenögger's avatar Hannes Rosenögger

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
parents dc6aa1d3 96644c1f
...@@ -2,6 +2,7 @@ class AutocompleteController < ApplicationController ...@@ -2,6 +2,7 @@ class AutocompleteController < ApplicationController
skip_before_action :authenticate_user!, only: [:users] skip_before_action :authenticate_user!, only: [:users]
def users def users
begin
@users = @users =
if params[:project_id].present? if params[:project_id].present?
project = Project.find(params[:project_id]) project = Project.find(params[:project_id])
...@@ -17,10 +18,18 @@ class AutocompleteController < ApplicationController ...@@ -17,10 +18,18 @@ class AutocompleteController < ApplicationController
end end
elsif current_user elsif current_user
User.all User.all
else end
User.none rescue ActiveRecord::RecordNotFound
if current_user
return render json: {}, status: 404
end
end
if @users.nil? && current_user.nil?
authenticate_user!
end end
@users ||= User.none
@users = @users.search(params[:search]) if params[:search].present? @users = @users.search(params[:search]) if params[:search].present?
@users = @users.active @users = @users.active
@users = @users.page(params[:page]).per(PER_PAGE) @users = @users.page(params[:page]).per(PER_PAGE)
......
...@@ -9,34 +9,58 @@ describe AutocompleteController do ...@@ -9,34 +9,58 @@ describe AutocompleteController do
before do before do
sign_in(user) sign_in(user)
project.team << [user, :master] project.team << [user, :master]
get(:users, project_id: project.id)
end end
let(:body) { JSON.parse(response.body) } let(:body) { JSON.parse(response.body) }
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).to be_kind_of(Array) }
it { expect(body.size).to eq 1 } it { expect(body.size).to eq 1 }
it { expect(body.first["username"]).to eq user.username } it { expect(body.first["username"]).to eq user.username }
end 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 context 'group members' do
let(:group) { create(:group) } let(:group) { create(:group) }
before do before do
sign_in(user) sign_in(user)
group.add_owner(user) group.add_owner(user)
get(:users, group_id: group.id)
end end
let(:body) { JSON.parse(response.body) } let(:body) { JSON.parse(response.body) }
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).to be_kind_of(Array) }
it { expect(body.size).to eq 1 } it { expect(body.size).to eq 1 }
it { expect(body.first["username"]).to eq user.username } it { expect(body.first["username"]).to eq user.username }
end 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 context 'all users' do
before do before do
sign_in(user) sign_in(user)
...@@ -50,26 +74,50 @@ describe AutocompleteController do ...@@ -50,26 +74,50 @@ describe AutocompleteController do
end end
context 'unauthenticated user' do context 'unauthenticated user' do
let(:project) { create(:project, :public) } let(:public_project) { create(:project, :public) }
let(:body) { JSON.parse(response.body) } let(:body) { JSON.parse(response.body) }
describe 'GET #users with public project' do describe 'GET #users with public project' do
before do before do
project.team << [user, :guest] public_project.team << [user, :guest]
get(:users, project_id: project.id) get(:users, project_id: public_project.id)
end end
it { expect(body).to be_kind_of(Array) } it { expect(body).to be_kind_of(Array) }
it { expect(body.size).to eq 1 } it { expect(body.size).to eq 1 }
end 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 describe 'GET #users with no project' do
before do before do
get(:users) get(:users)
end end
it { expect(body).to be_kind_of(Array) } it { expect(response.status).to eq(302) }
it { expect(body.size).to eq 0 }
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment