Commit 024e34e9 authored by Alex Lossent's avatar Alex Lossent

Hide passwords to non-admin users in the services API

In order to be consistent with !1490 doing it for the web interface
parent 5ffbf5fe
...@@ -45,6 +45,7 @@ v 8.1.0 (unreleased) ...@@ -45,6 +45,7 @@ v 8.1.0 (unreleased)
- Fix position of hamburger in header for smaller screens (Han Loong Liauw) - Fix position of hamburger in header for smaller screens (Han Loong Liauw)
- Fix bug where Emojis in Markdown would truncate remaining text (Sakata Sinji) - Fix bug where Emojis in Markdown would truncate remaining text (Sakata Sinji)
- Persist filters when sorting on admin user page (Jerry Lukins) - Persist filters when sorting on admin user page (Jerry Lukins)
- Hide passwords from services API (Alex Lossent)
v 8.0.4 v 8.0.4
- Fix Message-ID header to be RFC 2111-compliant to prevent e-mails being dropped (Stan Hu) - Fix Message-ID header to be RFC 2111-compliant to prevent e-mails being dropped (Stan Hu)
......
...@@ -255,6 +255,18 @@ module API ...@@ -255,6 +255,18 @@ module API
expose :notification_level expose :notification_level
end end
class ProjectService < Grape::Entity
expose :id, :title, :created_at, :updated_at, :active
expose :push_events, :issues_events, :merge_requests_events, :tag_push_events, :note_events
# Expose serialized properties
expose :properties do |service, options|
field_names = service.fields.
select { |field| options[:include_passwords] || field[:type] != 'password' }.
map { |field| field[:name] }
service.properties.slice(*field_names)
end
end
class ProjectWithAccess < Project class ProjectWithAccess < Project
expose :permissions do expose :permissions do
expose :project_access, using: Entities::ProjectAccess do |project, options| expose :project_access, using: Entities::ProjectAccess do |project, options|
......
...@@ -57,7 +57,7 @@ module API ...@@ -57,7 +57,7 @@ module API
# GET /project/:id/services/gitlab-ci # GET /project/:id/services/gitlab-ci
# #
get ':id/services/:service_slug' do get ':id/services/:service_slug' do
present project_service present project_service, with: Entities::ProjectService, include_passwords: current_user.is_admin?
end end
end end
end end
......
...@@ -3,6 +3,8 @@ require "spec_helper" ...@@ -3,6 +3,8 @@ require "spec_helper"
describe API::API, api: true do describe API::API, api: true do
include ApiHelpers include ApiHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:admin) { create(:admin) }
let(:user2) { create(:user) }
let(:project) {create(:project, creator_id: user.id, namespace: user.namespace) } let(:project) {create(:project, creator_id: user.id, namespace: user.namespace) }
Service.available_services_names.each do |service| Service.available_services_names.each do |service|
...@@ -51,11 +53,40 @@ describe API::API, api: true do ...@@ -51,11 +53,40 @@ describe API::API, api: true do
describe "GET /projects/:id/services/#{service.dasherize}" do describe "GET /projects/:id/services/#{service.dasherize}" do
include_context service include_context service
it "should get #{service} settings" do # inject some properties into the service
before do
project.build_missing_services
service_object = project.send(service_method)
service_object.properties = service_attrs
service_object.save
end
it 'should return authentication error when unauthenticated' do
get api("/projects/#{project.id}/services/#{dashed_service}")
expect(response.status).to eq(401)
end
it "should return all properties of service #{service} when authenticated as admin" do
get api("/projects/#{project.id}/services/#{dashed_service}", admin)
expect(response.status).to eq(200)
expect(json_response['properties'].keys.map(&:to_sym)).to match_array(service_attrs_list.map)
end
it "should return properties of service #{service} other than passwords when authenticated as project owner" do
get api("/projects/#{project.id}/services/#{dashed_service}", user) get api("/projects/#{project.id}/services/#{dashed_service}", user)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response['properties'].keys.map(&:to_sym)).to match_array(service_attrs_list_without_passwords)
end
it "should return error when authenticated but not a project owner" do
project.team << [user2, :developer]
get api("/projects/#{project.id}/services/#{dashed_service}", user2)
expect(response.status).to eq(403)
end end
end end
end end
end end
...@@ -3,7 +3,13 @@ Service.available_services_names.each do |service| ...@@ -3,7 +3,13 @@ Service.available_services_names.each do |service|
let(:dashed_service) { service.dasherize } let(:dashed_service) { service.dasherize }
let(:service_method) { "#{service}_service".to_sym } let(:service_method) { "#{service}_service".to_sym }
let(:service_klass) { "#{service}_service".classify.constantize } let(:service_klass) { "#{service}_service".classify.constantize }
let(:service_attrs_list) { service_klass.new.fields.inject([]) {|arr, hash| arr << hash[:name].to_sym } } let(:service_fields) { service_klass.new.fields }
let(:service_attrs_list) { service_fields.inject([]) {|arr, hash| arr << hash[:name].to_sym } }
let(:service_attrs_list_without_passwords) do
service_fields.
select { |field| field[:type] != 'password' }.
map { |field| field[:name].to_sym}
end
let(:service_attrs) do let(:service_attrs) do
service_attrs_list.inject({}) do |hash, k| service_attrs_list.inject({}) do |hash, k|
if k =~ /^(token*|.*_token|.*_key)/ if k =~ /^(token*|.*_token|.*_key)/
......
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