Commit dd826a5f authored by Z.J. van de Weg's avatar Z.J. van de Weg
Browse files

Return a consistent not found message

This prevents leakage of project names on an endpoint which is
unauthenticated and thus open to the world.
parent 9600a3988cf2
......@@ -77,7 +77,7 @@ def update_forks_visibility_level
has_one :last_event, -> {order 'events.created_at DESC'}, class_name: 'Event'
has_many :boards, before_add: :validate_board_limit, dependent: :destroy
has_many :chat_services, dependent: :destroy
has_many :chat_services
# Project services
has_one :campfire_service, dependent: :destroy
......
......@@ -98,14 +98,14 @@
t.text "help_page_text_html"
t.text "shared_runners_text_html"
t.text "after_sign_up_text_html"
t.boolean "sidekiq_throttling_enabled", default: false
t.string "sidekiq_throttling_queues"
t.decimal "sidekiq_throttling_factor"
t.boolean "housekeeping_enabled", default: true, null: false
t.boolean "housekeeping_bitmaps_enabled", default: true, null: false
t.integer "housekeeping_incremental_repack_period", default: 10, null: false
t.integer "housekeeping_full_repack_period", default: 50, null: false
t.integer "housekeeping_gc_period", default: 200, null: false
t.boolean "sidekiq_throttling_enabled", default: false
t.string "sidekiq_throttling_queues"
t.decimal "sidekiq_throttling_factor"
end
create_table "audit_events", force: :cascade do |t|
......@@ -384,6 +384,17 @@
add_index "ci_variables", ["gl_project_id"], name: "index_ci_variables_on_gl_project_id", using: :btree
create_table "custom_emoji", force: :cascade do |t|
t.integer "project_id", null: false
t.string "name"
t.string "emoji"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end
add_index "custom_emoji", ["project_id", "name"], name: "index_custom_emoji_on_project_id_and_name", unique: true, using: :btree
add_index "custom_emoji", ["project_id"], name: "index_custom_emoji_on_project_id", using: :btree
create_table "deploy_keys_projects", force: :cascade do |t|
t.integer "deploy_key_id", null: false
t.integer "project_id", null: false
......@@ -930,7 +941,7 @@
t.boolean "has_external_wiki"
t.boolean "lfs_enabled"
t.text "description_html"
t.boolean "only_allow_merge_if_all_discussions_are_resolved"
t.boolean "only_allow_merge_if_all_discussions_are_resolved", default: false, null: false
end
add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
......@@ -1254,6 +1265,7 @@
add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
add_foreign_key "boards", "projects"
add_foreign_key "custom_emoji", "projects"
add_foreign_key "issue_metrics", "issues", on_delete: :cascade
add_foreign_key "label_priorities", "labels", on_delete: :cascade
add_foreign_key "label_priorities", "projects", on_delete: :cascade
......
......@@ -67,7 +67,8 @@ class Services < Grape::API
post ':id/services/:service_slug/trigger' do
project = Project.find_with_namespace(params[:id]) || Project.find_by(id: params[:id])
not_found! unless project
# This is not accurate, but done to prevent leakage of the project names
not_found!('Service') unless project
service = project_service(project)
......@@ -77,7 +78,7 @@ class Services < Grape::API
status result[:status] || 200
present result
else
not_found!
not_found!('Service')
end
end
end
......
......@@ -14,8 +14,8 @@ def authorize_chat_name(url)
end
def help(commands, trigger)
if commands.empty?
ephemeral_response("No commands configured") unless messages.count > 1
if commands.zero?
ephemeral_response("No commands configured")
else
message = header_with_list("Available commands", commands)
......
......@@ -20,7 +20,7 @@
it { is_expected.to have_many(:deploy_keys) }
it { is_expected.to have_many(:hooks).dependent(:destroy) }
it { is_expected.to have_many(:protected_branches).dependent(:destroy) }
it { is_expected.to have_many(:chat_services).dependent(:destroy) }
it { is_expected.to have_many(:chat_services) }
it { is_expected.to have_one(:forked_project_link).dependent(:destroy) }
it { is_expected.to have_one(:slack_service).dependent(:destroy) }
it { is_expected.to have_one(:pushover_service).dependent(:destroy) }
......
......@@ -95,9 +95,10 @@
context 'no service is available' do
it 'returns a not found message' do
post api("/projects/#{project.id}/services/mattermost_command/trigger")
post api("/projects/#{project.id}/services/idonotexist/trigger")
expect(response).to have_http_status(404)
expect(json_response["message"]).to eq("404 Service Not Found")
end
end
......@@ -139,7 +140,7 @@
post api("/projects/404/services/mattermost_command/trigger"), params
expect(response).to have_http_status(404)
expect(json_response["message"]).to eq '404 Not Found'
expect(json_response["message"]).to eq("404 Service Not Found")
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