From dd826a5f20837f33263c658e41a4def0fc932069 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 18 Nov 2016 12:08:30 +0100 Subject: Return a consistent not found message This prevents leakage of project names on an endpoint which is unauthenticated and thus open to the world. --- app/models/project.rb | 2 +- db/schema.rb | 20 ++++++++++++++++---- lib/api/services.rb | 5 +++-- lib/mattermost/presenter.rb | 4 ++-- spec/models/project_spec.rb | 2 +- spec/requests/api/services_spec.rb | 5 +++-- 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 2c6b43bafdf..e6ae91f259d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -77,7 +77,7 @@ class Project < ActiveRecord::Base 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 diff --git a/db/schema.rb b/db/schema.rb index 8f8a03e1534..97a28ccad26 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -98,14 +98,14 @@ ActiveRecord::Schema.define(version: 20161113184239) do 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 @@ ActiveRecord::Schema.define(version: 20161113184239) do 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 @@ ActiveRecord::Schema.define(version: 20161113184239) do 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 @@ ActiveRecord::Schema.define(version: 20161113184239) do 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 diff --git a/lib/api/services.rb b/lib/api/services.rb index e3c6a998631..4d23499aa39 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -67,7 +67,8 @@ module 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 @@ module API status result[:status] || 200 present result else - not_found! + not_found!('Service') end end end diff --git a/lib/mattermost/presenter.rb b/lib/mattermost/presenter.rb index b4e7358770f..f76d0376a98 100644 --- a/lib/mattermost/presenter.rb +++ b/lib/mattermost/presenter.rb @@ -14,8 +14,8 @@ module Mattermost 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) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 08eb3bc9cd4..1972a8bb3f4 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -20,7 +20,7 @@ describe Project, models: true do 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) } diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 765d662e52b..782d76db318 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -95,9 +95,10 @@ describe API::API, api: true do 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 @@ describe API::API, api: true do 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 -- cgit v1.2.1