diff options
Diffstat (limited to 'lib')
32 files changed, 410 insertions, 239 deletions
diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 01e31f6f7d1..1942aeea656 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -433,11 +433,14 @@ module API end class LabelBasic < Grape::Entity - expose :name, :color, :description + expose :id, :name, :color, :description end class Label < LabelBasic expose :open_issues_count, :closed_issues_count, :open_merge_requests_count + expose :priority do |label, options| + label.priority(options[:project]) + end expose :subscribed do |label, options| label.subscribed?(options[:current_user]) diff --git a/lib/api/groups.rb b/lib/api/groups.rb index a13e353b7f5..40644fc2adf 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -26,6 +26,16 @@ module API present @groups, with: Entities::Group end + # Get list of owned groups for authenticated user + # + # Example Request: + # GET /groups/owned + get '/owned' do + @groups = current_user.owned_groups + @groups = paginate @groups + present @groups, with: Entities::Group, user: current_user + end + # Create group. Available only for users who can create groups. # # Parameters: diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 238cea00fba..97218054f37 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -11,7 +11,7 @@ module API success Entities::Label end get ':id/labels' do - present available_labels, with: Entities::Label, current_user: current_user + present available_labels, with: Entities::Label, current_user: current_user, project: user_project end desc 'Create a new label' do @@ -21,6 +21,7 @@ module API requires :name, type: String, desc: 'The name of the label to be created' requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" optional :description, type: String, desc: 'The description of label to be created' + optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true end post ':id/labels' do authorize! :admin_label, user_project @@ -28,10 +29,15 @@ module API label = available_labels.find_by(title: params[:name]) conflict!('Label already exists') if label - label = user_project.labels.create(declared(params, include_parent_namespaces: false).to_h) + priority = params.delete(:priority) + label_params = declared(params, + include_parent_namespaces: false, + include_missing: false).to_h + label = user_project.labels.create(label_params) if label.valid? - present label, with: Entities::Label, current_user: current_user + label.prioritize!(user_project, priority) if priority + present label, with: Entities::Label, current_user: current_user, project: user_project else render_validation_error!(label) end @@ -49,7 +55,7 @@ module API label = user_project.labels.find_by(title: params[:name]) not_found!('Label') unless label - present label.destroy, with: Entities::Label, current_user: current_user + present label.destroy, with: Entities::Label, current_user: current_user, project: user_project end desc 'Update an existing label. At least one optional parameter is required.' do @@ -60,7 +66,8 @@ module API optional :new_name, type: String, desc: 'The new name of the label' optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" optional :description, type: String, desc: 'The new description of label' - at_least_one_of :new_name, :color, :description + optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true + at_least_one_of :new_name, :color, :description, :priority end put ':id/labels' do authorize! :admin_label, user_project @@ -68,17 +75,25 @@ module API label = user_project.labels.find_by(title: params[:name]) not_found!('Label not found') unless label - update_params = declared(params, - include_parent_namespaces: false, - include_missing: false).to_h + update_priority = params.key?(:priority) + priority = params.delete(:priority) + label_params = declared(params, + include_parent_namespaces: false, + include_missing: false).to_h # Rename new name to the actual label attribute name - update_params['name'] = update_params.delete('new_name') if update_params.key?('new_name') + label_params[:name] = label_params.delete('new_name') if label_params.key?('new_name') - if label.update(update_params) - present label, with: Entities::Label, current_user: current_user - else - render_validation_error!(label) + render_validation_error!(label) unless label.update(label_params) + + if update_priority + if priority.nil? + label.unprioritize!(user_project) + else + label.prioritize!(user_project, priority) + end end + + present label, with: Entities::Label, current_user: current_user, project: user_project end end end diff --git a/lib/api/session.rb b/lib/api/session.rb index 55ec66a6d67..d09400b81f5 100644 --- a/lib/api/session.rb +++ b/lib/api/session.rb @@ -1,15 +1,14 @@ module API - # Users API class Session < Grape::API - # Login to get token - # - # Parameters: - # login (*required) - user login - # email (*required) - user email - # password (required) - user password - # - # Example Request: - # POST /session + desc 'Login to get token' do + success Entities::UserLogin + end + params do + optional :login, type: String, desc: 'The username' + optional :email, type: String, desc: 'The email of the user' + requires :password, type: String, desc: 'The password of the user' + at_least_one_of :login, :email + end post "/session" do user = Gitlab::Auth.find_with_user_password(params[:email] || params[:login], params[:password]) diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb index 32f731c5652..b6bfff9f20f 100644 --- a/lib/api/system_hooks.rb +++ b/lib/api/system_hooks.rb @@ -32,7 +32,7 @@ module API if hook.save present hook, with: Entities::Hook else - not_found! + render_validation_error!(hook) end end diff --git a/lib/api/users.rb b/lib/api/users.rb index c28e07a76b7..298c401a816 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -10,6 +10,9 @@ module API # GET /users # GET /users?search=Admin # GET /users?username=root + # GET /users?active=true + # GET /users?external=true + # GET /users?blocked=true get do unless can?(current_user, :read_users_list, nil) render_api_error!("Not authorized.", 403) @@ -19,8 +22,10 @@ module API @users = User.where(username: params[:username]) else @users = User.all - @users = @users.active if params[:active].present? + @users = @users.active if to_boolean(params[:active]) @users = @users.search(params[:search]) if params[:search].present? + @users = @users.blocked if to_boolean(params[:blocked]) + @users = @users.external if to_boolean(params[:external]) && current_user.is_admin? @users = paginate @users end diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb index 799b83b1069..80c844baecd 100644 --- a/lib/banzai/filter/autolink_filter.rb +++ b/lib/banzai/filter/autolink_filter.rb @@ -71,6 +71,14 @@ module Banzai @doc = parse_html(rinku) end + # Return true if any of the UNSAFE_PROTOCOLS strings are included in the URI scheme + def contains_unsafe?(scheme) + return false unless scheme + + scheme = scheme.strip.downcase + Banzai::Filter::SanitizationFilter::UNSAFE_PROTOCOLS.any? { |protocol| scheme.include?(protocol) } + end + # Autolinks any text matching LINK_PATTERN that Rinku didn't already # replace def text_parse @@ -89,17 +97,27 @@ module Banzai doc end - def autolink_filter(text) - text.gsub(LINK_PATTERN) do |match| - # Remove any trailing HTML entities and store them for appending - # outside the link element. The entity must be marked HTML safe in - # order to be output literally rather than escaped. - match.gsub!(/((?:&[\w#]+;)+)\z/, '') - dropped = ($1 || '').html_safe - - options = link_options.merge(href: match) - content_tag(:a, match, options) + dropped + def autolink_match(match) + # start by stripping out dangerous links + begin + uri = Addressable::URI.parse(match) + return match if contains_unsafe?(uri.scheme) + rescue Addressable::URI::InvalidURIError + return match end + + # Remove any trailing HTML entities and store them for appending + # outside the link element. The entity must be marked HTML safe in + # order to be output literally rather than escaped. + match.gsub!(/((?:&[\w#]+;)+)\z/, '') + dropped = ($1 || '').html_safe + + options = link_options.merge(href: match) + content_tag(:a, match, options) + dropped + end + + def autolink_filter(text) + text.gsub(LINK_PATTERN) { |match| autolink_match(match) } end def link_options diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index f5d110e987b..d8a855ec1fe 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -63,12 +63,7 @@ module Banzai nodes.select do |node| if node.has_attribute?(project_attr) node_id = node.attr(project_attr).to_i - - if project && project.id == node_id - true - else - can?(user, :read_project, projects[node_id]) - end + can_read_reference?(user, projects[node_id]) else true end @@ -226,6 +221,15 @@ module Banzai attr_reader :current_user, :project + # When a feature is disabled or visible only for + # team members we should not allow team members + # see reference comments. + # Override this method on subclasses + # to check if user can read resource + def can_read_reference?(user, ref_project) + raise NotImplementedError + end + def lazy(&block) Gitlab::Lazy.new(&block) end diff --git a/lib/banzai/reference_parser/commit_parser.rb b/lib/banzai/reference_parser/commit_parser.rb index 0fee9d267de..8c54a041cb8 100644 --- a/lib/banzai/reference_parser/commit_parser.rb +++ b/lib/banzai/reference_parser/commit_parser.rb @@ -29,6 +29,12 @@ module Banzai commits end + + private + + def can_read_reference?(user, ref_project) + can?(user, :download_code, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/commit_range_parser.rb b/lib/banzai/reference_parser/commit_range_parser.rb index 69d01f8db15..0878b6afba3 100644 --- a/lib/banzai/reference_parser/commit_range_parser.rb +++ b/lib/banzai/reference_parser/commit_range_parser.rb @@ -33,6 +33,12 @@ module Banzai range.valid_commits? ? range : nil end + + private + + def can_read_reference?(user, ref_project) + can?(user, :download_code, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/external_issue_parser.rb b/lib/banzai/reference_parser/external_issue_parser.rb index a1264db2111..6e7b7669578 100644 --- a/lib/banzai/reference_parser/external_issue_parser.rb +++ b/lib/banzai/reference_parser/external_issue_parser.rb @@ -20,6 +20,12 @@ module Banzai def issue_ids_per_project(nodes) gather_attributes_per_project(nodes, self.class.data_attribute) end + + private + + def can_read_reference?(user, ref_project) + can?(user, :read_issue, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/label_parser.rb b/lib/banzai/reference_parser/label_parser.rb index e5d1eb11d7f..aa76c64ac5f 100644 --- a/lib/banzai/reference_parser/label_parser.rb +++ b/lib/banzai/reference_parser/label_parser.rb @@ -6,6 +6,12 @@ module Banzai def references_relation Label end + + private + + def can_read_reference?(user, ref_project) + can?(user, :read_label, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb index c9a9ca79c09..40451947e6c 100644 --- a/lib/banzai/reference_parser/merge_request_parser.rb +++ b/lib/banzai/reference_parser/merge_request_parser.rb @@ -6,6 +6,12 @@ module Banzai def references_relation MergeRequest.includes(:author, :assignee, :target_project) end + + private + + def can_read_reference?(user, ref_project) + can?(user, :read_merge_request, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/milestone_parser.rb b/lib/banzai/reference_parser/milestone_parser.rb index a000ac61e5c..d3968d6b229 100644 --- a/lib/banzai/reference_parser/milestone_parser.rb +++ b/lib/banzai/reference_parser/milestone_parser.rb @@ -6,6 +6,12 @@ module Banzai def references_relation Milestone end + + private + + def can_read_reference?(user, ref_project) + can?(user, :read_milestone, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/snippet_parser.rb b/lib/banzai/reference_parser/snippet_parser.rb index fa71b3c952a..63b592137bb 100644 --- a/lib/banzai/reference_parser/snippet_parser.rb +++ b/lib/banzai/reference_parser/snippet_parser.rb @@ -6,6 +6,12 @@ module Banzai def references_relation Snippet end + + private + + def can_read_reference?(user, ref_project) + can?(user, :read_project_snippet, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb index 863f5725d3b..7adaffa19c1 100644 --- a/lib/banzai/reference_parser/user_parser.rb +++ b/lib/banzai/reference_parser/user_parser.rb @@ -30,22 +30,36 @@ module Banzai nodes.each do |node| if node.has_attribute?(group_attr) - node_group = groups[node.attr(group_attr).to_i] - - if node_group && - can?(user, :read_group, node_group) - visible << node - end - # Remaining nodes will be processed by the parent class' - # implementation of this method. + next unless can_read_group_reference?(node, user, groups) + visible << node + elsif can_read_project_reference?(node) + visible << node else remaining << node end end + # If project does not belong to a group + # and does not have the same project id as the current project + # base class will check if user can read the project that contains + # the user reference. visible + super(current_user, remaining) end + # Check if project belongs to a group which + # user can read. + def can_read_group_reference?(node, user, groups) + node_group = groups[node.attr('data-group').to_i] + + node_group && can?(user, :read_group, node_group) + end + + def can_read_project_reference?(node) + node_id = node.attr('data-project').to_i + + project && project.id == node_id + end + def nodes_user_can_reference(current_user, nodes) project_attr = 'data-project' author_attr = 'data-author' @@ -88,6 +102,10 @@ module Banzai collection_objects_for_ids(Project, ids). flat_map { |p| p.team.members.to_a } end + + def can_read_reference?(user, ref_project) + can?(user, :read_project, ref_project) + end end end end diff --git a/lib/constraints/constrainer_helper.rb b/lib/constraints/constrainer_helper.rb new file mode 100644 index 00000000000..ab07a6793d9 --- /dev/null +++ b/lib/constraints/constrainer_helper.rb @@ -0,0 +1,15 @@ +module ConstrainerHelper + def extract_resource_path(path) + id = path.dup + id.sub!(/\A#{relative_url_root}/, '') if relative_url_root + id.sub(/\A\/+/, '').sub(/\/+\z/, '').sub(/.atom\z/, '') + end + + private + + def relative_url_root + if defined?(Gitlab::Application.config.relative_url_root) + Gitlab::Application.config.relative_url_root + end + end +end diff --git a/lib/constraints/group_url_constrainer.rb b/lib/constraints/group_url_constrainer.rb index ca39b1961ae..2af6e1a11c8 100644 --- a/lib/constraints/group_url_constrainer.rb +++ b/lib/constraints/group_url_constrainer.rb @@ -1,7 +1,15 @@ -require 'constraints/namespace_url_constrainer' +require_relative 'constrainer_helper' -class GroupUrlConstrainer < NamespaceUrlConstrainer - def find_resource(id) - Group.find_by_path(id) +class GroupUrlConstrainer + include ConstrainerHelper + + def matches?(request) + id = extract_resource_path(request.path) + + if id =~ Gitlab::Regex.namespace_regex + Group.find_by(path: id).present? + else + false + end end end diff --git a/lib/constraints/namespace_url_constrainer.rb b/lib/constraints/namespace_url_constrainer.rb deleted file mode 100644 index 91b70143f11..00000000000 --- a/lib/constraints/namespace_url_constrainer.rb +++ /dev/null @@ -1,24 +0,0 @@ -class NamespaceUrlConstrainer - def matches?(request) - id = request.path - id = id.sub(/\A#{relative_url_root}/, '') if relative_url_root - id = id.sub(/\A\/+/, '').split('/').first - id = id.sub(/.atom\z/, '') if id - - if id =~ Gitlab::Regex.namespace_regex - find_resource(id) - end - end - - def find_resource(id) - Namespace.find_by_path(id) - end - - private - - def relative_url_root - if defined?(Gitlab::Application.config.relative_url_root) - Gitlab::Application.config.relative_url_root - end - end -end diff --git a/lib/constraints/user_url_constrainer.rb b/lib/constraints/user_url_constrainer.rb index 504a0f5d93e..4d722ad5af2 100644 --- a/lib/constraints/user_url_constrainer.rb +++ b/lib/constraints/user_url_constrainer.rb @@ -1,7 +1,15 @@ -require 'constraints/namespace_url_constrainer' +require_relative 'constrainer_helper' -class UserUrlConstrainer < NamespaceUrlConstrainer - def find_resource(id) - User.find_by('lower(username) = ?', id.downcase) +class UserUrlConstrainer + include ConstrainerHelper + + def matches?(request) + id = extract_resource_path(request.path) + + if id =~ Gitlab::Regex.namespace_regex + User.find_by('lower(username) = ?', id.downcase).present? + else + false + end end end diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 9b74364849e..82551f1f222 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -106,7 +106,7 @@ module ExtractsPath # resolved (e.g., when a user inserts an invalid path or ref). def assign_ref_vars # assign allowed options - allowed_options = ["filter_ref", "extended_sha1"] + allowed_options = ["filter_ref"] @options = params.select {|key, value| allowed_options.include?(key) && !value.blank? } @options = HashWithIndifferentAccess.new(@options) @@ -114,17 +114,13 @@ module ExtractsPath @ref, @path = extract_ref(@id) @repo = @project.repository - if @options[:extended_sha1].present? - @commit = @repo.commit(@options[:extended_sha1]) - else - @commit = @repo.commit(@ref) + @commit = @repo.commit(@ref) - if @path.empty? && !@commit && @id.ends_with?('.atom') - @id = @ref = extract_ref_without_atom(@id) - @commit = @repo.commit(@ref) + if @path.empty? && !@commit && @id.ends_with?('.atom') + @id = @ref = extract_ref_without_atom(@id) + @commit = @repo.commit(@ref) - request.format = :atom if @commit - end + request.format = :atom if @commit end raise InvalidPathError unless @commit diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index 9cec71a3222..82e194c1af1 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -127,19 +127,6 @@ module Gitlab 'rm-project', storage, "#{name}.git"]) end - # Gc repository - # - # storage - project storage path - # path - project path with namespace - # - # Ex. - # gc("/path/to/storage", "gitlab/gitlab-ci") - # - def gc(storage, path) - Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'gc', - storage, "#{path}.git"]) - end - # Add new key to gitlab-shell # # Ex. diff --git a/lib/gitlab/contributions_calendar.rb b/lib/gitlab/contributions_calendar.rb index b164f5a2eea..7e3d5647b39 100644 --- a/lib/gitlab/contributions_calendar.rb +++ b/lib/gitlab/contributions_calendar.rb @@ -1,45 +1,44 @@ module Gitlab class ContributionsCalendar - attr_reader :activity_dates, :projects, :user + attr_reader :contributor + attr_reader :current_user + attr_reader :projects - def initialize(projects, user) - @projects = projects - @user = user + def initialize(contributor, current_user = nil) + @contributor = contributor + @current_user = current_user + @projects = ContributedProjectsFinder.new(contributor).execute(current_user) end def activity_dates return @activity_dates if @activity_dates.present? - @activity_dates = {} + # Can't use Event.contributions here because we need to check 3 different + # project_features for the (currently) 3 different contribution types date_from = 1.year.ago + repo_events = event_counts(date_from, :repository). + having(action: Event::PUSHED) + issue_events = event_counts(date_from, :issues). + having(action: [Event::CREATED, Event::CLOSED], target_type: "Issue") + mr_events = event_counts(date_from, :merge_requests). + having(action: [Event::MERGED, Event::CREATED, Event::CLOSED], target_type: "MergeRequest") - events = Event.reorder(nil).contributions.where(author_id: user.id). - where("created_at > ?", date_from).where(project_id: projects). - group('date(created_at)'). - select('date(created_at) as date, count(id) as total_amount'). - map(&:attributes) + union = Gitlab::SQL::Union.new([repo_events, issue_events, mr_events]) + events = Event.find_by_sql(union.to_sql).map(&:attributes) - activity_dates = (1.year.ago.to_date..Date.today).to_a - - activity_dates.each do |date| - day_events = events.find { |day_events| day_events["date"] == date } - - if day_events - @activity_dates[date] = day_events["total_amount"] - end + @activity_events = events.each_with_object(Hash.new {|h, k| h[k] = 0 }) do |event, activities| + activities[event["date"]] += event["total_amount"] end - - @activity_dates end def events_by_date(date) - events = Event.contributions.where(author_id: user.id). - where("created_at > ? AND created_at < ?", date.beginning_of_day, date.end_of_day). + events = Event.contributions.where(author_id: contributor.id). + where(created_at: date.beginning_of_day..date.end_of_day). where(project_id: projects) - events.select do |event| - event.push? || event.issue? || event.merge_request? - end + # Use visible_to_user? instead of the complicated logic in activity_dates + # because we're only viewing the events for a single day. + events.select {|event| event.visible_to_user?(current_user) } end def starting_year @@ -49,5 +48,30 @@ module Gitlab def starting_month Date.today.month end + + private + + def event_counts(date_from, feature) + t = Event.arel_table + + # re-running the contributed projects query in each union is expensive, so + # use IN(project_ids...) instead. It's the intersection of two users so + # the list will be (relatively) short + @contributed_project_ids ||= projects.uniq.pluck(:id) + authed_projects = Project.where(id: @contributed_project_ids). + with_feature_available_for_user(feature, current_user). + reorder(nil). + select(:id) + + conditions = t[:created_at].gteq(date_from.beginning_of_day). + and(t[:created_at].lteq(Date.today.end_of_day)). + and(t[:author_id].eq(contributor.id)) + + Event.reorder(nil). + select(t[:project_id], t[:target_type], t[:action], 'date(created_at) AS date', 'count(id) as total_amount'). + group(t[:project_id], t[:target_type], t[:action], 'date(created_at)'). + where(conditions). + having(t[:project_id].in(Arel::Nodes::SqlLiteral.new(authed_projects.to_sql))) + end end end diff --git a/lib/gitlab/email/handler.rb b/lib/gitlab/email/handler.rb index 5cf9d5ebe28..bd3267e2a80 100644 --- a/lib/gitlab/email/handler.rb +++ b/lib/gitlab/email/handler.rb @@ -4,8 +4,7 @@ require 'gitlab/email/handler/create_issue_handler' module Gitlab module Email module Handler - # The `CreateIssueHandler` feature is disabled for the time being. - HANDLERS = [CreateNoteHandler] + HANDLERS = [CreateNoteHandler, CreateIssueHandler] def self.for(mail, mail_key) HANDLERS.find do |klass| diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb index 4e6566af8ab..9f90a3ec2b2 100644 --- a/lib/gitlab/email/handler/create_issue_handler.rb +++ b/lib/gitlab/email/handler/create_issue_handler.rb @@ -5,16 +5,16 @@ module Gitlab module Email module Handler class CreateIssueHandler < BaseHandler - attr_reader :project_path, :authentication_token + attr_reader :project_path, :incoming_email_token def initialize(mail, mail_key) super(mail, mail_key) - @project_path, @authentication_token = + @project_path, @incoming_email_token = mail_key && mail_key.split('+', 2) end def can_handle? - !authentication_token.nil? + !incoming_email_token.nil? end def execute @@ -29,7 +29,7 @@ module Gitlab end def author - @author ||= User.find_by(authentication_token: authentication_token) + @author ||= User.find_by(incoming_email_token: incoming_email_token) end def project diff --git a/lib/gitlab/exclusive_lease.rb b/lib/gitlab/exclusive_lease.rb index 7e8f35e9298..2dd42704396 100644 --- a/lib/gitlab/exclusive_lease.rb +++ b/lib/gitlab/exclusive_lease.rb @@ -1,66 +1,52 @@ +require 'securerandom' + module Gitlab # This class implements an 'exclusive lease'. We call it a 'lease' # because it has a set expiry time. We call it 'exclusive' because only # one caller may obtain a lease for a given key at a time. The # implementation is intended to work across GitLab processes and across - # servers. It is a 'cheap' alternative to using SQL queries and updates: + # servers. It is a cheap alternative to using SQL queries and updates: # you do not need to change the SQL schema to start using # ExclusiveLease. # - # It is important to choose the timeout wisely. If the timeout is very - # high (1 hour) then the throughput of your operation gets very low (at - # most once an hour). If the timeout is lower than how long your - # operation may take then you cannot count on exclusivity. For example, - # if the timeout is 10 seconds and you do an operation which may take 20 - # seconds then two overlapping operations may hold a lease for the same - # key at the same time. - # - # This class has no 'cancel' method. I originally decided against adding - # it because it would add complexity and a false sense of security. The - # complexity: instead of setting '1' we would have to set a UUID, and to - # delete it we would have to execute Lua on the Redis server to only - # delete the key if the value was our own UUID. Otherwise there is a - # chance that when you intend to cancel your lease you actually delete - # someone else's. The false sense of security: you cannot design your - # system to rely too much on the lease being cancelled after use because - # the calling (Ruby) process may crash or be killed. You _cannot_ count - # on begin/ensure blocks to cancel a lease, because the 'ensure' does - # not always run. Think of 'kill -9' from the Unicorn master for - # instance. - # - # If you find that leases are getting in your way, ask yourself: would - # it be enough to lower the lease timeout? Another thing that might be - # appropriate is to only use a lease for bulk/automated operations, and - # to ignore the lease when you get a single 'manual' user request (a - # button click). - # class ExclusiveLease + LUA_CANCEL_SCRIPT = <<-EOS + local key, uuid = KEYS[1], ARGV[1] + if redis.call("get", key) == uuid then + redis.call("del", key) + end + EOS + + def self.cancel(key, uuid) + Gitlab::Redis.with do |redis| + redis.eval(LUA_CANCEL_SCRIPT, keys: [redis_key(key)], argv: [uuid]) + end + end + + def self.redis_key(key) + "gitlab:exclusive_lease:#{key}" + end + def initialize(key, timeout:) - @key, @timeout = key, timeout + @redis_key = self.class.redis_key(key) + @timeout = timeout + @uuid = SecureRandom.uuid end - # Try to obtain the lease. Return true on success, + # Try to obtain the lease. Return lease UUID on success, # false if the lease is already taken. def try_obtain # Performing a single SET is atomic Gitlab::Redis.with do |redis| - !!redis.set(redis_key, '1', nx: true, ex: @timeout) + redis.set(@redis_key, @uuid, nx: true, ex: @timeout) && @uuid end end # Returns true if the key for this lease is set. def exists? Gitlab::Redis.with do |redis| - redis.exists(redis_key) + redis.exists(@redis_key) end end - - # No #cancel method. See comments above! - - private - - def redis_key - "gitlab:exclusive_lease:#{@key}" - end end end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 799794c0171..bcbf6455998 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -2,8 +2,18 @@ # class return an instance of `GitlabAccessStatus` module Gitlab class GitAccess + UnauthorizedError = Class.new(StandardError) + + ERROR_MESSAGES = { + upload: 'You are not allowed to upload code for this project.', + download: 'You are not allowed to download code from this project.', + deploy_key: 'Deploy keys are not allowed to push code.', + no_repo: 'A repository for this project does not exist yet.' + } + DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive } PUSH_COMMANDS = %w{ git-receive-pack } + ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS attr_reader :actor, :project, :protocol, :user_access, :authentication_abilities @@ -16,56 +26,43 @@ module Gitlab end def check(cmd, changes) - return build_status_object(false, "Git access over #{protocol.upcase} is not allowed") unless protocol_allowed? - - unless actor - return build_status_object(false, "No user or key was provided.") - end - - if user && !user_access.allowed? - return build_status_object(false, "Your account has been blocked.") - end - - unless project && (user_access.can_read_project? || deploy_key_can_read_project?) - return build_status_object(false, 'The project you were looking for could not be found.') - end + check_protocol! + check_active_user! + check_project_accessibility! + check_command_existence!(cmd) case cmd when *DOWNLOAD_COMMANDS download_access_check when *PUSH_COMMANDS push_access_check(changes) - else - build_status_object(false, "The command you're trying to execute is not allowed.") end + + build_status_object(true) + rescue UnauthorizedError => ex + build_status_object(false, ex.message) end def download_access_check if user user_download_access_check - elsif deploy_key - build_status_object(true) - else - raise 'Wrong actor' + elsif deploy_key.nil? && !Guest.can?(:download_code, project) + raise UnauthorizedError, ERROR_MESSAGES[:download] end end def push_access_check(changes) if user user_push_access_check(changes) - elsif deploy_key - build_status_object(false, "Deploy keys are not allowed to push code.") else - raise 'Wrong actor' + raise UnauthorizedError, ERROR_MESSAGES[deploy_key ? :deploy_key : :upload] end end def user_download_access_check unless user_can_download_code? || build_can_download_code? - return build_status_object(false, "You are not allowed to download code from this project.") + raise UnauthorizedError, ERROR_MESSAGES[:download] end - - build_status_object(true) end def user_can_download_code? @@ -78,15 +75,15 @@ module Gitlab def user_push_access_check(changes) unless authentication_abilities.include?(:push_code) - return build_status_object(false, "You are not allowed to upload code for this project.") + raise UnauthorizedError, ERROR_MESSAGES[:upload] end if changes.blank? - return build_status_object(true) + return # Allow access. end unless project.repository.exists? - return build_status_object(false, "A repository for this project does not exist yet.") + raise UnauthorizedError, ERROR_MESSAGES[:no_repo] end changes_list = Gitlab::ChangesList.new(changes) @@ -96,11 +93,9 @@ module Gitlab status = change_access_check(change) unless status.allowed? # If user does not have access to make at least one change - cancel all push - return status + raise UnauthorizedError, status.message end end - - build_status_object(true) end def change_access_check(change) @@ -113,6 +108,30 @@ module Gitlab private + def check_protocol! + unless protocol_allowed? + raise UnauthorizedError, "Git access over #{protocol.upcase} is not allowed" + end + end + + def check_active_user! + if user && !user_access.allowed? + raise UnauthorizedError, "Your account has been blocked." + end + end + + def check_project_accessibility! + if project.blank? || !can_read_project? + raise UnauthorizedError, 'The project you were looking for could not be found.' + end + end + + def check_command_existence!(cmd) + unless ALL_COMMANDS.include?(cmd) + raise UnauthorizedError, "The command you're trying to execute is not allowed." + end + end + def matching_merge_request?(newrev, branch_name) Checks::MatchingMergeRequest.new(newrev, branch_name, project).match? end @@ -130,6 +149,16 @@ module Gitlab end end + def can_read_project? + if user + user_access.can_read_project? + elsif deploy_key + deploy_key_can_read_project? + else + Guest.can?(:read_project, project) + end + end + protected def user diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb index d7be50bd437..801dfde9a36 100644 --- a/lib/gitlab/incoming_email.rb +++ b/lib/gitlab/incoming_email.rb @@ -1,5 +1,7 @@ module Gitlab module IncomingEmail + WILDCARD_PLACEHOLDER = '%{key}'.freeze + class << self FALLBACK_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze @@ -7,8 +9,16 @@ module Gitlab config.enabled && config.address end + def supports_wildcard? + config.address && config.address.include?(WILDCARD_PLACEHOLDER) + end + + def supports_issue_creation? + enabled? && supports_wildcard? + end + def reply_address(key) - config.address.gsub('%{key}', key) + config.address.gsub(WILDCARD_PLACEHOLDER, key) end def key_from_address(address) diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb index f9bb5775323..6ea069d26df 100644 --- a/lib/gitlab/ldap/config.rb +++ b/lib/gitlab/ldap/config.rb @@ -92,6 +92,10 @@ module Gitlab options['timeout'].to_i end + def has_auth? + options['password'] || options['bind_dn'] + end + protected def base_config @@ -122,10 +126,6 @@ module Gitlab } } end - - def has_auth? - options['password'] || options['bind_dn'] - end end end end diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 24733435a5a..b8326a64b22 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -5,11 +5,7 @@ module Gitlab def initialize(current_user, project, query, repository_ref = nil) @current_user = current_user @project = project - @repository_ref = if repository_ref.present? - repository_ref - else - nil - end + @repository_ref = repository_ref.presence @query = query end @@ -47,33 +43,31 @@ module Gitlab private def blobs - if project.empty_repo? || query.blank? - [] - else - project.repository.search_files(query, repository_ref) - end + @blobs ||= project.repository.search_files(query, repository_ref) end def wiki_blobs - if project.wiki_enabled? && query.present? - project_wiki = ProjectWiki.new(project) + @wiki_blobs ||= begin + if project.wiki_enabled? && query.present? + project_wiki = ProjectWiki.new(project) - unless project_wiki.empty? - project_wiki.search_files(query) + unless project_wiki.empty? + project_wiki.search_files(query) + else + [] + end else [] end - else - [] end end def notes - project.notes.user.search(query, as_user: @current_user).order('updated_at DESC') + @notes ||= project.notes.user.search(query, as_user: @current_user).order('updated_at DESC') end def commits - project.repository.find_commits_by_message(query) + @commits ||= project.repository.find_commits_by_message(query) end def project_ids_relation diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 0d30e1bb92e..cb1659f9cee 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -8,6 +8,10 @@ module Gitlab @namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze end + def namespace_route_regex + @namespace_route_regex ||= /#{NAMESPACE_REGEX_STR}/.freeze + end + def namespace_regex_message "can contain only letters, digits, '_', '-' and '.'. " \ "Cannot start with '-' or end in '.', '.git' or '.atom'." \ diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 2ae48a970ce..35c4194e87c 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -760,7 +760,7 @@ namespace :gitlab do end namespace :ldap do - task :check, [:limit] => :environment do |t, args| + task :check, [:limit] => :environment do |_, args| # Only show up to 100 results because LDAP directories can be very big. # This setting only affects the `rake gitlab:check` script. args.with_defaults(limit: 100) @@ -768,7 +768,7 @@ namespace :gitlab do start_checking "LDAP" if Gitlab::LDAP::Config.enabled? - print_users(args.limit) + check_ldap(args.limit) else puts 'LDAP is disabled in config/gitlab.yml' end @@ -776,21 +776,42 @@ namespace :gitlab do finished_checking "LDAP" end - def print_users(limit) - puts "LDAP users with access to your GitLab server (only showing the first #{limit} results)" - + def check_ldap(limit) servers = Gitlab::LDAP::Config.providers servers.each do |server| puts "Server: #{server}" - Gitlab::LDAP::Adapter.open(server) do |adapter| - users = adapter.users(adapter.config.uid, '*', limit) - users.each do |user| - puts "\tDN: #{user.dn}\t #{adapter.config.uid}: #{user.uid}" + + begin + Gitlab::LDAP::Adapter.open(server) do |adapter| + check_ldap_auth(adapter) + + puts "LDAP users with access to your GitLab server (only showing the first #{limit} results)" + + users = adapter.users(adapter.config.uid, '*', limit) + users.each do |user| + puts "\tDN: #{user.dn}\t #{adapter.config.uid}: #{user.uid}" + end end + rescue Net::LDAP::ConnectionRefusedError, Errno::ECONNREFUSED => e + puts "Could not connect to the LDAP server: #{e.message}".color(:red) end end end + + def check_ldap_auth(adapter) + auth = adapter.config.has_auth? + + if auth && adapter.ldap.bind + message = 'Success'.color(:green) + elsif auth + message = 'Failed. Check `bind_dn` and `password` configuration values'.color(:red) + else + message = 'Anonymous. No `bind_dn` or `password` configured'.color(:yellow) + end + + puts "LDAP authentication... #{message}" + end end namespace :repo do |