Commit f40b5860 authored by Reuben Pereira's avatar Reuben Pereira Committed by Sean McGivern

Add table and model for error tracking settings

parent 549ee8ad
# frozen_string_literal: true
module ErrorTracking
class ProjectErrorTrackingSetting < ActiveRecord::Base
belongs_to :project
validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true }
attr_encrypted :token,
mode: :per_attribute_iv,
key: Settings.attr_encrypted_db_key_base_truncated,
algorithm: 'aes-256-gcm'
end
end
...@@ -187,6 +187,7 @@ class Project < ActiveRecord::Base ...@@ -187,6 +187,7 @@ class Project < ActiveRecord::Base
has_one :import_state, autosave: true, class_name: 'ProjectImportState', inverse_of: :project has_one :import_state, autosave: true, class_name: 'ProjectImportState', inverse_of: :project
has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_one :project_repository, inverse_of: :project has_one :project_repository, inverse_of: :project
has_one :error_tracking_setting, inverse_of: :project, class_name: 'ErrorTracking::ProjectErrorTrackingSetting'
# Merge Requests for target project should be removed with it # Merge Requests for target project should be removed with it
has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
# - allow_local_network: Allow urls pointing to private network addresses. Default: true # - allow_local_network: Allow urls pointing to private network addresses. Default: true
# - ports: Allowed ports. Default: all. # - ports: Allowed ports. Default: all.
# - enforce_user: Validate user format. Default: false # - enforce_user: Validate user format. Default: false
# - enforce_sanitization: Validate that there are no html/css/js tags. Default: false
# #
# Example: # Example:
# class User < ActiveRecord::Base # class User < ActiveRecord::Base
...@@ -70,7 +71,8 @@ class UrlValidator < ActiveModel::EachValidator ...@@ -70,7 +71,8 @@ class UrlValidator < ActiveModel::EachValidator
allow_localhost: true, allow_localhost: true,
allow_local_network: true, allow_local_network: true,
ascii_only: false, ascii_only: false,
enforce_user: false enforce_user: false,
enforce_sanitization: false
} }
end end
......
# frozen_string_literal: true
class CreateErrorTrackingSettings < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
create_table :project_error_tracking_settings, id: :int, primary_key: :project_id, default: nil do |t|
t.boolean :enabled, null: false, default: true
t.string :api_url, null: false
t.string :encrypted_token
t.string :encrypted_token_iv
t.foreign_key :projects, column: :project_id, on_delete: :cascade
end
end
end
...@@ -1573,6 +1573,13 @@ ActiveRecord::Schema.define(version: 20190103140724) do ...@@ -1573,6 +1573,13 @@ ActiveRecord::Schema.define(version: 20190103140724) do
t.index ["project_id", "deploy_token_id"], name: "index_project_deploy_tokens_on_project_id_and_deploy_token_id", unique: true, using: :btree t.index ["project_id", "deploy_token_id"], name: "index_project_deploy_tokens_on_project_id_and_deploy_token_id", unique: true, using: :btree
end end
create_table "project_error_tracking_settings", primary_key: "project_id", id: :integer, force: :cascade do |t|
t.boolean "enabled", default: true, null: false
t.string "api_url", null: false
t.string "encrypted_token"
t.string "encrypted_token_iv"
end
create_table "project_features", force: :cascade do |t| create_table "project_features", force: :cascade do |t|
t.integer "project_id", null: false t.integer "project_id", null: false
t.integer "merge_requests_access_level" t.integer "merge_requests_access_level"
...@@ -2434,6 +2441,7 @@ ActiveRecord::Schema.define(version: 20190103140724) do ...@@ -2434,6 +2441,7 @@ ActiveRecord::Schema.define(version: 20190103140724) do
add_foreign_key "project_custom_attributes", "projects", on_delete: :cascade add_foreign_key "project_custom_attributes", "projects", on_delete: :cascade
add_foreign_key "project_deploy_tokens", "deploy_tokens", on_delete: :cascade add_foreign_key "project_deploy_tokens", "deploy_tokens", on_delete: :cascade
add_foreign_key "project_deploy_tokens", "projects", on_delete: :cascade add_foreign_key "project_deploy_tokens", "projects", on_delete: :cascade
add_foreign_key "project_error_tracking_settings", "projects", on_delete: :cascade
add_foreign_key "project_features", "projects", name: "fk_18513d9b92", on_delete: :cascade add_foreign_key "project_features", "projects", name: "fk_18513d9b92", on_delete: :cascade
add_foreign_key "project_group_links", "projects", name: "fk_daa8cee94c", on_delete: :cascade add_foreign_key "project_group_links", "projects", name: "fk_daa8cee94c", on_delete: :cascade
add_foreign_key "project_import_data", "projects", name: "fk_ffb9ee3a10", on_delete: :cascade add_foreign_key "project_import_data", "projects", name: "fk_ffb9ee3a10", on_delete: :cascade
......
...@@ -74,6 +74,7 @@ project_tree: ...@@ -74,6 +74,7 @@ project_tree:
- :prometheus_metrics - :prometheus_metrics
- :project_badges - :project_badges
- :ci_cd_settings - :ci_cd_settings
- :error_tracking_setting
# Only include the following attributes for the models specified. # Only include the following attributes for the models specified.
included_attributes: included_attributes:
...@@ -162,6 +163,9 @@ excluded_attributes: ...@@ -162,6 +163,9 @@ excluded_attributes:
- :token_encrypted - :token_encrypted
services: services:
- :template - :template
error_tracking_setting:
- :encrypted_token
- :encrypted_token_iv
methods: methods:
labels: labels:
......
...@@ -24,6 +24,7 @@ module Gitlab ...@@ -24,6 +24,7 @@ module Gitlab
project_badges: 'Badge', project_badges: 'Badge',
metrics: 'MergeRequest::Metrics', metrics: 'MergeRequest::Metrics',
ci_cd_settings: 'ProjectCiCdSetting', ci_cd_settings: 'ProjectCiCdSetting',
error_tracking_setting: 'ErrorTracking::ProjectErrorTrackingSetting',
links: 'Releases::Link' }.freeze links: 'Releases::Link' }.freeze
USER_REFERENCES = %w[author_id assignee_id updated_by_id merged_by_id latest_closed_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id closed_by_id].freeze USER_REFERENCES = %w[author_id assignee_id updated_by_id merged_by_id latest_closed_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id closed_by_id].freeze
......
...@@ -8,16 +8,18 @@ module Gitlab ...@@ -8,16 +8,18 @@ module Gitlab
BlockedUrlError = Class.new(StandardError) BlockedUrlError = Class.new(StandardError)
class << self class << self
def validate!(url, ports: [], protocols: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false) def validate!(url, ports: [], protocols: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false, enforce_sanitization: false)
return true if url.nil? return true if url.nil?
# Param url can be a string, URI or Addressable::URI # Param url can be a string, URI or Addressable::URI
uri = parse_url(url) uri = parse_url(url)
validate_html_tags!(uri) if enforce_sanitization
# Allow imports from the GitLab instance itself but only from the configured ports # Allow imports from the GitLab instance itself but only from the configured ports
return true if internal?(uri) return true if internal?(uri)
port = uri.port || uri.default_port port = get_port(uri)
validate_protocol!(uri.scheme, protocols) validate_protocol!(uri.scheme, protocols)
validate_port!(port, ports) if ports.any? validate_port!(port, ports) if ports.any?
validate_user!(uri.user) if enforce_user validate_user!(uri.user) if enforce_user
...@@ -50,6 +52,18 @@ module Gitlab ...@@ -50,6 +52,18 @@ module Gitlab
private private
def get_port(uri)
uri.port || uri.default_port
end
def validate_html_tags!(uri)
uri_str = uri.to_s
sanitized_uri = ActionController::Base.helpers.sanitize(uri_str, tags: [])
if sanitized_uri != uri_str
raise BlockedUrlError, 'HTML/CSS/JS tags are not allowed'
end
end
def parse_url(url) def parse_url(url)
raise Addressable::URI::InvalidURIError if multiline?(url) raise Addressable::URI::InvalidURIError if multiline?(url)
......
...@@ -64,6 +64,7 @@ describe 'Database schema' do ...@@ -64,6 +64,7 @@ describe 'Database schema' do
let(:indexes) { connection.indexes(table) } let(:indexes) { connection.indexes(table) }
let(:columns) { connection.columns(table) } let(:columns) { connection.columns(table) }
let(:foreign_keys) { connection.foreign_keys(table) } let(:foreign_keys) { connection.foreign_keys(table) }
let(:primary_key_column) { connection.primary_key(table) }
context 'all foreign keys' do context 'all foreign keys' do
# for index to be effective, the FK constraint has to be at first place # for index to be effective, the FK constraint has to be at first place
...@@ -71,6 +72,12 @@ describe 'Database schema' do ...@@ -71,6 +72,12 @@ describe 'Database schema' do
first_indexed_column = indexes.map(&:columns).map(&:first) first_indexed_column = indexes.map(&:columns).map(&:first)
foreign_keys_columns = foreign_keys.map(&:column) foreign_keys_columns = foreign_keys.map(&:column)
# Add the primary key column to the list of indexed columns because
# postgres and mysql both automatically create an index on the primary
# key. Also, the rails connection.indexes() method does not return
# automatically generated indexes (like the primary key index).
first_indexed_column = first_indexed_column.push(primary_key_column)
expect(first_indexed_column.uniq).to include(*foreign_keys_columns) expect(first_indexed_column.uniq).to include(*foreign_keys_columns)
end end
end end
......
# frozen_string_literal: true
FactoryBot.define do
factory :project_error_tracking_setting, class: ErrorTracking::ProjectErrorTrackingSetting do
project
api_url 'https://gitlab.com'
enabled true
token 'access_token_123'
end
end
...@@ -314,6 +314,7 @@ project: ...@@ -314,6 +314,7 @@ project:
- repository_languages - repository_languages
- pool_repository - pool_repository
- kubernetes_namespaces - kubernetes_namespaces
- error_tracking_setting
award_emoji: award_emoji:
- awardable - awardable
- user - user
...@@ -345,3 +346,5 @@ resource_label_events: ...@@ -345,3 +346,5 @@ resource_label_events:
- merge_request - merge_request
- epic - epic
- label - label
error_tracking_setting:
- project
...@@ -600,3 +600,8 @@ ResourceLabelEvent: ...@@ -600,3 +600,8 @@ ResourceLabelEvent:
- label_id - label_id
- user_id - user_id
- created_at - created_at
ErrorTracking::ProjectErrorTrackingSetting:
- id
- api_url
- enabled
- project_id
# frozen_string_literal: true
require 'spec_helper'
describe ErrorTracking::ProjectErrorTrackingSetting do
set(:project) { create(:project) }
describe 'Associations' do
it { is_expected.to belong_to(:project) }
end
describe 'Validations' do
subject { create(:project_error_tracking_setting, project: project) }
context 'when api_url is over 255 chars' do
before do
subject.api_url = 'https://' + 'a' * 250
end
it 'fails validation' do
expect(subject).not_to be_valid
expect(subject.errors.messages[:api_url]).to include('is too long (maximum is 255 characters)')
end
end
context 'With unsafe url' do
let(:project_error_tracking_setting) { create(:project_error_tracking_setting, project: project) }
it 'fails validation' do
project_error_tracking_setting.api_url = "https://replaceme.com/'><script>alert(document.cookie)</script>"
expect(project_error_tracking_setting).not_to be_valid
end
end
end
end
...@@ -62,6 +62,7 @@ describe Project do ...@@ -62,6 +62,7 @@ describe Project do
it { is_expected.to have_one(:last_event).class_name('Event') } it { is_expected.to have_one(:last_event).class_name('Event') }
it { is_expected.to have_one(:forked_from_project).through(:fork_network_member) } it { is_expected.to have_one(:forked_from_project).through(:fork_network_member) }
it { is_expected.to have_one(:auto_devops).class_name('ProjectAutoDevops') } it { is_expected.to have_one(:auto_devops).class_name('ProjectAutoDevops') }
it { is_expected.to have_one(:error_tracking_setting).class_name('ErrorTracking::ProjectErrorTrackingSetting') }
it { is_expected.to have_many(:commit_statuses) } it { is_expected.to have_many(:commit_statuses) }
it { is_expected.to have_many(:ci_pipelines) } it { is_expected.to have_many(:ci_pipelines) }
it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:builds) }
......
...@@ -172,4 +172,55 @@ describe UrlValidator do ...@@ -172,4 +172,55 @@ describe UrlValidator do
end end
end end
end end
context 'when enforce_sanitization is' do
let(:validator) { described_class.new(attributes: [:link_url], enforce_sanitization: enforce_sanitization) }
let(:unsafe_url) { "https://replaceme.com/'><script>alert(document.cookie)</script>" }
let(:safe_url) { 'https://replaceme.com/path/to/somewhere' }
let(:unsafe_internal_url) do
Gitlab.config.gitlab.protocol + '://' + Gitlab.config.gitlab.host +
"/'><script>alert(document.cookie)</script>"
end
context 'true' do
let(:enforce_sanitization) { true }
it 'prevents unsafe urls' do
badge.link_url = unsafe_url
subject
expect(badge.errors.empty?).to be false
end
it 'prevents unsafe internal urls' do
badge.link_url = unsafe_internal_url
subject
expect(badge.errors.empty?).to be false
end
it 'allows safe urls' do
badge.link_url = safe_url
subject
expect(badge.errors.empty?).to be true
end
end
context 'false' do
let(:enforce_sanitization) { false }
it 'allows unsafe urls' do
badge.link_url = unsafe_url
subject
expect(badge.errors.empty?).to be true
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