Commit 0424801e authored by Stan Hu's avatar Stan Hu Committed by Stan Hu

Merge branch...

Merge branch 'security-10-3-do-not-expose-passwords-or-tokens-in-service-integrations-api' into 'security-10-3'

Filter out sensitive fields from the project services API

See merge request gitlab/gitlabhq!2281

(cherry picked from commit 476f2576444632f2a9a61b4cead9c1077f2c81d7)

2bcbbda0 Filter out sensitive fields from the project services API
parent 3228ac06
...@@ -118,6 +118,11 @@ class Service < ActiveRecord::Base ...@@ -118,6 +118,11 @@ class Service < ActiveRecord::Base
nil nil
end end
def api_field_names
fields.map { |field| field[:name] }
.reject { |field_name| field_name =~ /(password|token|key)/ }
end
def global_fields def global_fields
fields fields
end end
......
---
title: Filter out sensitive fields from the project services API
merge_request:
author: Robert Schilling
type: security
...@@ -714,10 +714,7 @@ module API ...@@ -714,10 +714,7 @@ module API
expose :job_events expose :job_events
# Expose serialized properties # Expose serialized properties
expose :properties do |service, options| expose :properties do |service, options|
field_names = service.fields service.properties.slice(*service.api_field_names)
.select { |field| options[:include_passwords] || field[:type] != 'password' }
.map { |field| field[:name] }
service.properties.slice(*field_names)
end end
end end
......
...@@ -785,7 +785,7 @@ module API ...@@ -785,7 +785,7 @@ module API
service_params = declared_params(include_missing: false).merge(active: true) service_params = declared_params(include_missing: false).merge(active: true)
if service.update_attributes(service_params) if service.update_attributes(service_params)
present service, with: Entities::ProjectService, include_passwords: current_user.admin? present service, with: Entities::ProjectService
else else
render_api_error!('400 Bad Request', 400) render_api_error!('400 Bad Request', 400)
end end
......
...@@ -257,10 +257,7 @@ module API ...@@ -257,10 +257,7 @@ module API
expose :job_events, as: :build_events expose :job_events, as: :build_events
# Expose serialized properties # Expose serialized properties
expose :properties do |service, options| expose :properties do |service, options|
field_names = service.fields service.properties.slice(*service.api_field_names)
.select { |field| options[:include_passwords] || field[:type] != 'password' }
.map { |field| field[:name] }
service.properties.slice(*field_names)
end end
end end
......
...@@ -622,7 +622,7 @@ module API ...@@ -622,7 +622,7 @@ module API
end end
get ":id/services/:service_slug" do get ":id/services/:service_slug" do
service = user_project.find_or_initialize_service(params[:service_slug].underscore) service = user_project.find_or_initialize_service(params[:service_slug].underscore)
present service, with: Entities::ProjectService, include_passwords: current_user.admin? present service, with: Entities::ProjectService
end end
end end
......
...@@ -255,6 +255,7 @@ describe Service do ...@@ -255,6 +255,7 @@ describe Service do
end end
end end
<<<<<<< HEAD
describe "#deprecated?" do describe "#deprecated?" do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
...@@ -278,6 +279,39 @@ describe Service do ...@@ -278,6 +279,39 @@ describe Service do
it 'returns service template' do it 'returns service template' do
expect(KubernetesService.find_by_template).to eq(kubernetes_service) expect(KubernetesService.find_by_template).to eq(kubernetes_service)
=======
describe '#api_field_names' do
let(:fake_service) do
Class.new(Service) do
def fields
[
{ name: 'token' },
{ name: 'api_token' },
{ name: 'key' },
{ name: 'api_key' },
{ name: 'password' },
{ name: 'password_field' },
{ name: 'safe_field' }
]
end
end
end
let(:service) do
fake_service.new(properties: [
{ token: 'token-value' },
{ api_token: 'api_token-value' },
{ key: 'key-value' },
{ api_key: 'api_key-value' },
{ password: 'password-value' },
{ password_field: 'password_field-value' },
{ safe_field: 'safe_field-value' }
])
end
it 'filters out sensitive fields' do
expect(service.api_field_names).to eq(['safe_field'])
>>>>>>> Merge branch 'security-10-3-do-not-expose-passwords-or-tokens-in-service-integrations-api' into 'security-10-3'
end end
end end
end end
...@@ -83,14 +83,14 @@ describe API::Services do ...@@ -83,14 +83,14 @@ describe API::Services do
get api("/projects/#{project.id}/services/#{dashed_service}", admin) get api("/projects/#{project.id}/services/#{dashed_service}", admin)
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response['properties'].keys.map(&:to_sym)).to match_array(service_attrs_list.map) expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
end end
it "returns properties of service #{service} other than passwords when authenticated as project owner" do it "returns 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).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response['properties'].keys.map(&:to_sym)).to match_array(service_attrs_list_without_passwords) expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
end end
it "returns error when authenticated but not a project owner" do it "returns error when authenticated but not a project owner" do
......
...@@ -3,13 +3,9 @@ Service.available_services_names.each do |service| ...@@ -3,13 +3,9 @@ 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_fields) { service_klass.new.fields } let(:service_instance) { service_klass.new }
let(:service_fields) { service_instance.fields }
let(:service_attrs_list) { service_fields.inject([]) {|arr, hash| arr << hash[:name].to_sym } } 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