diff --git a/CHANGELOG.md b/CHANGELOG.md index f47814ab3b..e11eb68240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,43 @@ Please view this file on the master branch, on stable branches it's out of date. +## 8.13.6 (2016-11-17) + +- Omniauth auto link LDAP user falls back to find by DN when user cannot be found by UID. !7002 +- Fix no "Register" tab if ldap auth is enabled (#24038). !7274 (Luc Didry) +- Fix cache for commit status in commits list to respect branches. !7372 +- Fix issue causing Labels not to appear in sidebar on MR page. !7416 (Alex Sanford) +- Limit labels returned for a specific project as an administrator. !7496 +- Clicking "force remove source branch" label now toggles the checkbox again. +- Allow commit note to be visible if repo is visible. +- Fix project Visibility Level selector not using default values. + +## 8.13.5 (2016-11-08) + +- Restore unauthenticated access to public container registries + +## 8.13.4 (2016-11-07) + +- Fix showing pipeline status for a given commit from correct branch. !7034 +- Only skip group when it's actually a group in the "Share with group" select. !7262 +- Introduce round-robin project creation to spread load over multiple shards. !7266 +- Ensure merge request's "remove branch" accessors return booleans. !7267 +- Ensure external users are not able to clone disabled repositories. +- Fix XSS issue in Markdown autolinker. +- Respect event visibility in Gitlab::ContributionsCalendar. +- Honour issue and merge request visibility in their respective finders. +- Disable reference Markdown for unavailable features. +- Fix lightweight tags not processed correctly by GitTagPushService. !6532 +- Allow owners to fetch source code in CI builds. !6943 +- Return conflict error in label API when title is taken by group label. !7014 +- Reduce the overhead to calculate number of open/closed issues and merge requests within the group or project. !7123 +- Fix builds tab visibility. !7178 +- Fix project features default values. !7181 + +## 8.13.3 (2016-11-02) + +- Removes any symlinks before importing a project export file. CVE-2016-9086 +- Fixed Import/Export foreign key issue to do with project members. + ## 8.13.2 (2016-10-31) - Fix encoding issues on pipeline commits. !6832 diff --git a/Gemfile b/Gemfile index 46245ab62d..5a95fdc82a 100644 --- a/Gemfile +++ b/Gemfile @@ -51,7 +51,7 @@ gem 'browser', '~> 2.2' # Extracting information from a git repository # Provide access to Gitlab::Git library -gem 'gitlab_git', '~> 10.6.8' +gem 'gitlab_git', '~> 10.7.0' # LDAP Auth # GitLab fork with several improvements to original library. For full list of changes diff --git a/Gemfile.lock b/Gemfile.lock index 442184b922..26c4dc0d23 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -283,7 +283,7 @@ GEM mime-types (>= 1.16, < 3) posix-spawn (~> 0.3) gitlab-markup (1.5.0) - gitlab_git (10.6.8) + gitlab_git (10.7.0) activesupport (~> 4.0) charlock_holmes (~> 0.7.3) github-linguist (~> 4.7.0) @@ -867,7 +867,7 @@ DEPENDENCIES github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.5.0) - gitlab_git (~> 10.6.8) + gitlab_git (~> 10.7.0) gitlab_omniauth-ldap (~> 1.2.1) gollum-lib (~> 4.2) gollum-rugged_adapter (~> 0.4.2) @@ -994,4 +994,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.13.2 + 1.13.5 diff --git a/VERSION b/VERSION index 3b3df40190..6b63235a86 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.13.3 +8.13.6 diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 8a61669822..58f7adf276 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -127,19 +127,30 @@ return $(document).off('scroll'); }; - window.shiftWindow = function() { - return scrollBy(0, -100); - }; - document.addEventListener("page:fetch", unbindEvents); - window.addEventListener("hashchange", shiftWindow); + // automatically adjust scroll position for hash urls taking the height of the navbar into account + // https://github.com/twitter/bootstrap/issues/1768 + window.adjustScroll = function() { + var navbar = document.querySelector('.navbar-gitlab'); + var subnav = document.querySelector('.layout-nav'); + var fixedTabs = document.querySelector('.js-tabs-affix'); - window.onload = function() { + adjustment = 0; + if (navbar) adjustment -= navbar.offsetHeight; + if (subnav) adjustment -= subnav.offsetHeight; + if (fixedTabs) adjustment -= fixedTabs.offsetHeight; + + return scrollBy(0, adjustment); + }; + + window.addEventListener("hashchange", adjustScroll); + + window.onload = function () { // Scroll the window to avoid the topnav bar // https://github.com/twitter/bootstrap/issues/1768 if (location.hash) { - return setTimeout(shiftWindow, 100); + return setTimeout(adjustScroll, 100); } }; diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 3dde979185..d282b2da2d 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -129,7 +129,7 @@ MergeRequestTabs.prototype.scrollToElement = function(container) { var $el, navBarHeight; if (window.location.hash) { - navBarHeight = $('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight(); + navBarHeight = $('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight() + document.querySelector('.js-tabs-affix').offsetHeight; $el = $(container + " " + window.location.hash + ":not(.match)"); if ($el.length) { return $.scrollTo(container + " " + window.location.hash + ":not(.match)", { diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 6ef7cf0bae..86e808314f 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -116,8 +116,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :metrics_packet_size, :send_user_confirmation_email, :container_registry_token_expire_delay, - :repository_storage, :enabled_git_access_protocol, + repository_storages: [], restricted_visibility_levels: [], import_sources: [], disabled_oauth_sign_in_sources: [] diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 37600ed875..517ad4f03f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -192,9 +192,10 @@ class ApplicationController < ActionController::Base end # JSON for infinite scroll via Pager object - def pager_json(partial, count) + def pager_json(partial, count, locals = {}) html = render_to_string( partial, + locals: locals, layout: false, formats: [:html] ) diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 7e4da73bc1..c736200a10 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -12,7 +12,7 @@ class JwtController < ApplicationController return head :not_found unless service result = service.new(@authentication_result.project, @authentication_result.actor, auth_params). - execute(authentication_abilities: @authentication_result.authentication_abilities || []) + execute(authentication_abilities: @authentication_result.authentication_abilities) render json: result, status: result[:http_status] end @@ -20,7 +20,7 @@ class JwtController < ApplicationController private def authenticate_project_or_user - @authentication_result = Gitlab::Auth::Result.new + @authentication_result = Gitlab::Auth::Result.new(nil, nil, :none, Gitlab::Auth.read_authentication_abilities) authenticate_with_http_basic do |login, password| @authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, ip: request.ip) diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index a52c614b25..4f38b2202f 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -26,8 +26,15 @@ class Projects::CommitsController < Projects::ApplicationController respond_to do |format| format.html - format.json { pager_json("projects/commits/_commits", @commits.size) } format.atom { render layout: false } + + format.json do + pager_json( + 'projects/commits/_commits', + @commits.size, + project: @project, + ref: @ref) + end end end end diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 383e184d79..3f41916e6d 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -21,10 +21,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController def authenticate_user @authentication_result = Gitlab::Auth::Result.new - if project && project.public? && download_request? - return # Allow access - end - if allow_basic_auth? && basic_auth_provided? login, password = user_name_and_password(request) @@ -41,6 +37,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController send_final_spnego_response return # Allow access end + elsif project && download_request? && Guest.can?(:download_code, project) + @authentication_result = Gitlab::Auth::Result.new(nil, project, :none, [:download_code]) + + return # Allow access end send_challenges diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 662d38b10a..13caeb42d4 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -78,11 +78,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController def upload_pack_allowed? return false unless Gitlab.config.gitlab_shell.upload_pack - if user - access_check.allowed? - else - ci? || project.public? - end + access_check.allowed? || ci? end def access diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb index ae060abee5..9eaf26a0db 100644 --- a/app/controllers/projects/group_links_controller.rb +++ b/app/controllers/projects/group_links_controller.rb @@ -7,7 +7,7 @@ class Projects::GroupLinksController < Projects::ApplicationController @group_links = project.project_group_links.all @skip_groups = @group_links.pluck(:group_id) - @skip_groups << project.group.try(:id) + @skip_groups << project.namespace_id unless project.personal? end def create diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 4f85513436..42fd09e9b7 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -126,7 +126,7 @@ class Projects::LabelsController < Projects::ApplicationController alias_method :subscribable_resource, :label def find_labels - @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute.includes(:priorities) + @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute end def authorize_admin_labels! diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb index ece49dcd92..2d49327694 100644 --- a/app/controllers/projects/lfs_api_controller.rb +++ b/app/controllers/projects/lfs_api_controller.rb @@ -31,10 +31,6 @@ class Projects::LfsApiController < Projects::GitHttpClientController private - def objects - @objects ||= (params[:objects] || []).to_a - end - def existing_oids @existing_oids ||= begin storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 2ee53f7ced..6e15c06c12 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -352,13 +352,23 @@ class Projects::MergeRequestsController < Projects::ApplicationController def branch_from # This is always source @source_project = @merge_request.nil? ? @project : @merge_request.source_project - @commit = @repository.commit(params[:ref]) if params[:ref].present? + + if params[:ref].present? + @ref = params[:ref] + @commit = @repository.commit(@ref) + end + render layout: false end def branch_to @target_project = selected_target_project - @commit = @target_project.commit(params[:ref]) if params[:ref].present? + + if params[:ref].present? + @ref = params[:ref] + @commit = @target_project.commit(@ref) + end + render layout: false end @@ -497,6 +507,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.close end + labels define_pipelines_vars end diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index 8fea20cefe..953091492a 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -23,7 +23,7 @@ class Projects::TagsController < Projects::ApplicationController return render_404 unless @tag @release = @project.releases.find_or_initialize_by(tag: @tag.name) - @commit = @repository.commit(@tag.target) + @commit = @repository.commit(@tag.dereferenced_target) end def create diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 76b730198d..fbe93e5dda 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -287,7 +287,8 @@ class ProjectsController < Projects::ApplicationController render 'projects/empty' if @project.empty_repo? else if @project.wiki_enabled? - @wiki_home = @project.wiki.find_page('home', params[:version_id]) + @project_wiki = @project.wiki + @wiki_home = @project_wiki.find_page('home', params[:version_id]) elsif @project.feature_available?(:issues, current_user) @issues = issues_collection @issues = @issues.page(params[:page]) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 6a881b271d..c4508ccc3b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -104,8 +104,7 @@ class UsersController < ApplicationController end def contributions_calendar - @contributions_calendar ||= Gitlab::ContributionsCalendar. - new(contributed_projects, user) + @contributions_calendar ||= Gitlab::ContributionsCalendar.new(user, current_user) end def load_events diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index e27986ef95..6297b2db36 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -61,31 +61,26 @@ class IssuableFinder def project return @project if defined?(@project) - if project? - @project = Project.find(params[:project_id]) + project = Project.find(params[:project_id]) + project = nil unless Ability.allowed?(current_user, :"read_#{klass.to_ability_name}", project) - unless Ability.allowed?(current_user, :read_project, @project) - @project = nil - end - else - @project = nil - end - - @project + @project = project end def projects return @projects if defined?(@projects) + return @projects = project if project? - if project? - @projects = project - elsif current_user && params[:authorized_only].presence && !current_user_related? - @projects = current_user.authorized_projects.reorder(nil) - elsif group - @projects = GroupProjectsFinder.new(group).execute(current_user).reorder(nil) - else - @projects = ProjectsFinder.new.execute(current_user).reorder(nil) - end + projects = + if current_user && params[:authorized_only].presence && !current_user_related? + current_user.authorized_projects + elsif group + GroupProjectsFinder.new(group).execute(current_user) + else + ProjectsFinder.new.execute(current_user) + end + + @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil) end def search @@ -126,7 +121,7 @@ class IssuableFinder @labels = if labels? && !filter_by_no_label? - LabelsFinder.new(current_user, project_ids: projects, title: label_names).execute + LabelsFinder.new(current_user, project_ids: projects, title: label_names).execute(skip_authorization: true) else Label.none end @@ -273,7 +268,7 @@ class IssuableFinder items = items.with_label(label_names, params[:sort]) if projects - label_ids = LabelsFinder.new(current_user, project_ids: projects).execute.select(:id) + label_ids = LabelsFinder.new(current_user, project_ids: projects).execute(skip_authorization: true).select(:id) items = items.where(labels: { id: label_ids }) end end diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 865f093f04..fa0e2a5e3d 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -6,7 +6,7 @@ class LabelsFinder < UnionFinder def execute(skip_authorization: false) @skip_authorization = skip_authorization - items = find_union(label_ids, Label) + items = find_union(label_ids, Label) || Label.none items = with_title(items) sort(items) end @@ -18,9 +18,11 @@ class LabelsFinder < UnionFinder def label_ids label_ids = [] - if project - label_ids << project.group.labels if project.group.present? - label_ids << project.labels + if project? + if project + label_ids << project.group.labels if project.group.present? + label_ids << project.labels + end else label_ids << Label.where(group_id: projects.group_ids) label_ids << Label.where(project_id: projects.select(:id)) @@ -40,16 +42,16 @@ class LabelsFinder < UnionFinder items.where(title: title) end - def group_id - params[:group_id].presence + def group? + params[:group_id].present? end - def project_id - params[:project_id].presence + def project? + params[:project_id].present? end - def projects_ids - params[:project_ids] + def projects? + params[:project_ids].present? end def title @@ -59,8 +61,9 @@ class LabelsFinder < UnionFinder def project return @project if defined?(@project) - if project_id - @project = find_project + if project? + @project = Project.find(params[:project_id]) + @project = nil unless authorized_to_read_labels?(@project) else @project = nil end @@ -68,26 +71,20 @@ class LabelsFinder < UnionFinder @project end - def find_project - if skip_authorization - Project.find_by(id: project_id) - else - available_projects.find_by(id: project_id) - end - end - def projects return @projects if defined?(@projects) - @projects = skip_authorization ? Project.all : available_projects - @projects = @projects.in_namespace(group_id) if group_id - @projects = @projects.where(id: projects_ids) if projects_ids + @projects = skip_authorization ? Project.all : ProjectsFinder.new.execute(current_user) + @projects = @projects.in_namespace(params[:group_id]) if group? + @projects = @projects.where(id: params[:project_ids]) if projects? @projects = @projects.reorder(nil) @projects end - def available_projects - @available_projects ||= ProjectsFinder.new.execute(current_user) + def authorized_to_read_labels?(project) + return true if skip_authorization + + Ability.allowed?(current_user, :read_label, project) end end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 6229384817..45a567a1eb 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -93,11 +93,11 @@ module ApplicationSettingsHelper end end - def repository_storage_options_for_select + def repository_storages_options_for_select options = Gitlab.config.repositories.storages.map do |name, path| ["#{name} - #{path}", name] end - options_for_select(options, @application_setting.repository_storage) + options_for_select(options, @application_setting.repository_storages) end end diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index b7f48630bd..3decedace4 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -54,10 +54,18 @@ module CiStatusHelper custom_icon(icon_name) end - def render_commit_status(commit, tooltip_placement: 'auto left') + def render_commit_status(commit, ref: nil, tooltip_placement: 'auto left') project = commit.project - path = pipelines_namespace_project_commit_path(project.namespace, project, commit) - render_status_with_link('commit', commit.status, path, tooltip_placement: tooltip_placement) + path = pipelines_namespace_project_commit_path( + project.namespace, + project, + commit) + + render_status_with_link( + 'commit', + commit.status(ref), + path, + tooltip_placement: tooltip_placement) end def render_pipeline_status(pipeline, tooltip_placement: 'auto left') diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 33dcee49ae..ed402b698f 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -25,9 +25,11 @@ module CommitsHelper end end - def commit_to_html(commit, project, inline = true) - template = inline ? "inline_commit" : "commit" - render "projects/commits/#{template}", commit: commit, project: project unless commit.nil? + def commit_to_html(commit, ref, project) + render 'projects/commits/commit', + commit: commit, + ref: ref, + project: project end # Breadcrumb links for a Project and, if applicable, a tree path diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index f8ded05c31..6d8b8b5dfa 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -80,7 +80,7 @@ module EventsHelper elsif event.merge_request? namespace_project_merge_request_url(event.project.namespace, event.project, event.merge_request) - elsif event.note? && event.commit_note? + elsif event.commit_note? namespace_project_commit_url(event.project.namespace, event.project, event.note_target) elsif event.note? @@ -121,7 +121,7 @@ module EventsHelper end def event_note_target_path(event) - if event.note? && event.commit_note? + if event.commit_note? namespace_project_commit_path(event.project.namespace, event.project, event.note_target, diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb index 95b60aeab5..2425c3a8bc 100644 --- a/app/helpers/lfs_helper.rb +++ b/app/helpers/lfs_helper.rb @@ -1,6 +1,6 @@ module LfsHelper include Gitlab::Routing.url_helpers - + def require_lfs_enabled! return if Gitlab.config.lfs.enabled @@ -27,7 +27,11 @@ module LfsHelper def lfs_download_access? return false unless project.lfs_enabled? - project.public? || ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code? + ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code? + end + + def objects + @objects ||= (params[:objects] || []).to_a end def user_can_download_code? diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index d26b4018be..42c00ec3cd 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -174,10 +174,14 @@ module ProjectsHelper nav_tabs << :merge_requests end - if can?(current_user, :read_build, project) + if can?(current_user, :read_pipeline, project) nav_tabs << :pipelines end + if can?(current_user, :read_build, project) + nav_tabs << :builds + end + if Gitlab.config.registry.enabled && can?(current_user, :read_container_image, project) nav_tabs << :container_registry end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index c99aa7772b..9b02a68a20 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -18,6 +18,7 @@ class ApplicationSetting < ActiveRecord::Base serialize :disabled_oauth_sign_in_sources, Array serialize :domain_whitelist, Array serialize :domain_blacklist, Array + serialize :repository_storages cache_markdown_field :sign_in_text cache_markdown_field :help_page_text @@ -74,9 +75,8 @@ class ApplicationSetting < ActiveRecord::Base presence: true, numericality: { only_integer: true, greater_than: 0 } - validates :repository_storage, - presence: true, - inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } } + validates :repository_storages, presence: true + validate :check_repository_storages validates :enabled_git_access_protocol, inclusion: { in: %w(ssh http), allow_blank: true, allow_nil: true } @@ -166,7 +166,7 @@ class ApplicationSetting < ActiveRecord::Base disabled_oauth_sign_in_sources: [], send_user_confirmation_email: false, container_registry_token_expire_delay: 5, - repository_storage: 'default', + repository_storages: ['default'], user_default_external: false, ) end @@ -201,6 +201,25 @@ class ApplicationSetting < ActiveRecord::Base self.domain_blacklist_raw = file.read end + def repository_storages + Array(read_attribute(:repository_storages)) + end + + # repository_storage is still required in the API. Remove in 9.0 + def repository_storage + repository_storages.first + end + + def repository_storage=(value) + self.repository_storages = [value] + end + + # Choose one of the available repository storage options. Currently all have + # equal weighting. + def pick_repository_storage + repository_storages.sample + end + def runners_registration_token ensure_runners_registration_token! end @@ -208,4 +227,12 @@ class ApplicationSetting < ActiveRecord::Base def health_check_access_token ensure_health_check_access_token! end + + private + + def check_repository_storages + invalid = repository_storages - Gitlab.config.repositories.storages.keys + errors.add(:repository_storages, "can't include: #{invalid.join(", ")}") unless + invalid.empty? + end end diff --git a/app/models/commit.rb b/app/models/commit.rb index e64fd1e0c1..9e7fde9503 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -226,12 +226,19 @@ class Commit end def pipelines - @pipeline ||= project.pipelines.where(sha: sha) + project.pipelines.where(sha: sha) end - def status - return @status if defined?(@status) - @status ||= pipelines.status + def status(ref = nil) + @statuses ||= {} + + if @statuses.key?(ref) + @statuses[ref] + elsif ref + @statuses[ref] = pipelines.where(ref: ref).status + else + @statuses[ref] = pipelines.status + end end def revert_branch_name diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 17c3b526c9..ce5f1deba2 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -182,6 +182,10 @@ module Issuable grouping_columns end + + def to_ability_name + model_name.singular + end end def today? @@ -243,7 +247,7 @@ module Issuable # issuable.class # => MergeRequest # issuable.to_ability_name # => "merge_request" def to_ability_name - self.class.to_s.underscore + self.class.to_ability_name end # Returns a Hash of attributes to be used for Twitter card metadata diff --git a/app/models/concerns/project_features_compatibility.rb b/app/models/concerns/project_features_compatibility.rb index 9216122923..6d88951c71 100644 --- a/app/models/concerns/project_features_compatibility.rb +++ b/app/models/concerns/project_features_compatibility.rb @@ -31,7 +31,7 @@ module ProjectFeaturesCompatibility def write_feature_attribute(field, value) build_project_feature unless project_feature - access_level = value == "true" ? ProjectFeature::ENABLED : ProjectFeature::DISABLED + access_level = Gitlab::Utils.to_boolean(value) ? ProjectFeature::ENABLED : ProjectFeature::DISABLED project_feature.update_attribute(field, access_level) end end diff --git a/app/models/event.rb b/app/models/event.rb index 0764cb8cab..1d193de1d3 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -48,6 +48,7 @@ class Event < ActiveRecord::Base update_all(updated_at: Time.now) end + # Update Gitlab::ContributionsCalendar#activity_dates if this changes def contributions where("action = ? OR (target_type in (?) AND action in (?))", Event::PUSHED, ["MergeRequest", "Issue"], @@ -60,8 +61,8 @@ class Event < ActiveRecord::Base end def visible_to_user?(user = nil) - if push? - true + if push? || commit_note? + Ability.allowed?(user, :download_code, project) elsif membership_changed? true elsif created_project? @@ -275,7 +276,7 @@ class Event < ActiveRecord::Base end def commit_note? - target.for_commit? + note? && target && target.for_commit? end def issue_note? @@ -287,7 +288,7 @@ class Event < ActiveRecord::Base end def project_snippet_note? - target.for_snippet? + note? && target && target.for_snippet? end def note_target diff --git a/app/models/group_label.rb b/app/models/group_label.rb index a698b532d1..68841ace2e 100644 --- a/app/models/group_label.rb +++ b/app/models/group_label.rb @@ -5,6 +5,10 @@ class GroupLabel < Label alias_attribute :subject, :group + def subject_foreign_key + 'group_id' + end + def to_reference(source_project = nil, target_project = nil, format: :id) super(source_project, target_project, format: format) end diff --git a/app/models/guest.rb b/app/models/guest.rb new file mode 100644 index 0000000000..01285ca126 --- /dev/null +++ b/app/models/guest.rb @@ -0,0 +1,7 @@ +class Guest + class << self + def can?(action, subject) + Ability.allowed?(nil, action, subject) + end + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index 133a599381..f0fe32187e 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -245,31 +245,11 @@ class Issue < ActiveRecord::Base # Returns `true` if the current issue can be viewed by either a logged in User # or an anonymous user. def visible_to_user?(user = nil) + return false unless project.feature_available?(:issues, user) + user ? readable_by?(user) : publicly_visible? end - # Returns `true` if the given User can read the current Issue. - def readable_by?(user) - if user.admin? - true - elsif project.owner == user - true - elsif confidential? - author == user || - assignee == user || - project.team.member?(user, Gitlab::Access::REPORTER) - else - project.public? || - project.internal? && !user.external? || - project.team.member?(user) - end - end - - # Returns `true` if this Issue is visible to everybody. - def publicly_visible? - project.public? && !confidential? - end - def overdue? due_date.try(:past?) || false end @@ -290,4 +270,32 @@ class Issue < ActiveRecord::Base end end end + + private + + # Returns `true` if the given User can read the current Issue. + # + # This method duplicates the same check of issue_policy.rb + # for performance reasons, check commit: 002ad215818450d2cbbc5fa065850a953dc7ada8 + # Make sure to sync this method with issue_policy.rb + def readable_by?(user) + if user.admin? + true + elsif project.owner == user + true + elsif confidential? + author == user || + assignee == user || + project.team.member?(user, Gitlab::Access::REPORTER) + else + project.public? || + project.internal? && !user.external? || + project.team.member?(user) + end + end + + # Returns `true` if this Issue is visible to everybody. + def publicly_visible? + project.public? && !confidential? + end end diff --git a/app/models/label.rb b/app/models/label.rb index 149fd98ecb..d9287f2dc2 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -92,16 +92,23 @@ class Label < ActiveRecord::Base nil end - def open_issues_count(user = nil, project = nil) - issues_count(user, project_id: project.try(:id) || project_id, state: 'opened') + def open_issues_count(user = nil) + issues_count(user, state: 'opened') end - def closed_issues_count(user = nil, project = nil) - issues_count(user, project_id: project.try(:id) || project_id, state: 'closed') + def closed_issues_count(user = nil) + issues_count(user, state: 'closed') end - def open_merge_requests_count(user = nil, project = nil) - merge_requests_count(user, project_id: project.try(:id) || project_id, state: 'opened') + def open_merge_requests_count(user = nil) + params = { + subject_foreign_key => subject.id, + label_name: title, + scope: 'all', + state: 'opened' + } + + MergeRequestsFinder.new(user, params.with_indifferent_access).execute.count end def prioritize!(project, value) @@ -167,15 +174,8 @@ class Label < ActiveRecord::Base end def issues_count(user, params = {}) - IssuesFinder.new(user, params.reverse_merge(label_name: title, scope: 'all')) - .execute - .count - end - - def merge_requests_count(user, params = {}) - MergeRequestsFinder.new(user, params.reverse_merge(label_name: title, scope: 'all')) - .execute - .count + params.merge!(subject_foreign_key => subject.id, label_name: title, scope: 'all') + IssuesFinder.new(user, params.with_indifferent_access).execute.count end def label_format_reference(format = :id) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index c476a3bb14..7663e055f0 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -442,11 +442,11 @@ class MergeRequest < ActiveRecord::Base end def should_remove_source_branch? - merge_params['should_remove_source_branch'].present? + Gitlab::Utils.to_boolean(merge_params['should_remove_source_branch']) end def force_remove_source_branch? - merge_params['force_remove_source_branch'].present? + Gitlab::Utils.to_boolean(merge_params['force_remove_source_branch']) end def remove_source_branch? diff --git a/app/models/project.rb b/app/models/project.rb index a5c1d5c9e3..e0062f3dbb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -28,8 +28,13 @@ class Project < ActiveRecord::Base default_value_for :archived, false default_value_for :visibility_level, gitlab_config_features.visibility_level default_value_for :container_registry_enabled, gitlab_config_features.container_registry - default_value_for(:repository_storage) { current_application_settings.repository_storage } + default_value_for(:repository_storage) { current_application_settings.pick_repository_storage } default_value_for(:shared_runners_enabled) { current_application_settings.shared_runners_enabled } + default_value_for :issues_enabled, gitlab_config_features.issues + default_value_for :merge_requests_enabled, gitlab_config_features.merge_requests + default_value_for :builds_enabled, gitlab_config_features.builds + default_value_for :wiki_enabled, gitlab_config_features.wiki + default_value_for :snippets_enabled, gitlab_config_features.snippets after_create :ensure_dir_exist after_create :create_project_feature, unless: :project_feature @@ -202,8 +207,38 @@ class Project < ActiveRecord::Base scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct } scope :with_push, -> { joins(:events).where('events.action = ?', Event::PUSHED) } - scope :with_builds_enabled, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id').where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0') } - scope :with_issues_enabled, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id').where('project_features.issues_access_level IS NULL or project_features.issues_access_level > 0') } + scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') } + + # "enabled" here means "not disabled". It includes private features! + scope :with_feature_enabled, ->(feature) { + access_level_attribute = ProjectFeature.access_level_attribute(feature) + with_project_feature.where(project_features: { access_level_attribute => [nil, ProjectFeature::PRIVATE, ProjectFeature::ENABLED] }) + } + + # Picks a feature where the level is exactly that given. + scope :with_feature_access_level, ->(feature, level) { + access_level_attribute = ProjectFeature.access_level_attribute(feature) + with_project_feature.where(project_features: { access_level_attribute => level }) + } + + scope :with_builds_enabled, -> { with_feature_enabled(:builds) } + scope :with_issues_enabled, -> { with_feature_enabled(:issues) } + + # project features may be "disabled", "internal" or "enabled". If "internal", + # they are only available to team members. This scope returns projects where + # the feature is either enabled, or internal with permission for the user. + def self.with_feature_available_for_user(feature, user) + return with_feature_enabled(feature) if user.try(:admin?) + + unconditional = with_feature_access_level(feature, [nil, ProjectFeature::ENABLED]) + return unconditional if user.nil? + + conditional = with_feature_access_level(feature, ProjectFeature::PRIVATE) + authorized = user.authorized_projects.merge(conditional.reorder(nil)) + + union = Gitlab::SQL::Union.new([unconditional.select(:id), authorized.select(:id)]) + where(arel_table[:id].in(Arel::Nodes::SqlLiteral.new(union.to_sql))) + end scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') } scope :abandoned, -> { where('projects.last_activity_at < ?', 6.months.ago) } @@ -390,7 +425,7 @@ class Project < ActiveRecord::Base end def group_ids - joins(:namespace).where(namespaces: { type: 'Group' }).pluck(:namespace_id) + joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id) end end @@ -1062,10 +1097,6 @@ class Project < ActiveRecord::Base forks.count end - def find_label(name) - labels.find_by(name: name) - end - def origin_merge_requests merge_requests.where(source_project_id: self.id) end diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index b37ce1d3cf..34fd5a57b5 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -20,6 +20,15 @@ class ProjectFeature < ActiveRecord::Base FEATURES = %i(issues merge_requests wiki snippets builds repository) + class << self + def access_level_attribute(feature) + feature = feature.model_name.plural.to_sym if feature.respond_to?(:model_name) + raise ArgumentError, "invalid project feature: #{feature}" unless FEATURES.include?(feature) + + "#{feature}_access_level".to_sym + end + end + # Default scopes force us to unscope here since a service may need to check # permissions for a project in pending_delete # http://stackoverflow.com/questions/1540645/how-to-disable-default-scope-for-a-belongs-to @@ -35,9 +44,8 @@ class ProjectFeature < ActiveRecord::Base default_value_for :repository_access_level, value: ENABLED, allows_nil: false def feature_available?(feature, user) - raise ArgumentError, 'invalid project feature' unless FEATURES.include?(feature) - - get_permission(user, public_send("#{feature}_access_level")) + access_level = public_send(ProjectFeature.access_level_attribute(feature)) + get_permission(user, access_level) end def builds_enabled? diff --git a/app/models/project_label.rb b/app/models/project_label.rb index 33c2b61771..82f47f0e8f 100644 --- a/app/models/project_label.rb +++ b/app/models/project_label.rb @@ -12,6 +12,10 @@ class ProjectLabel < Label alias_attribute :subject, :project + def subject_foreign_key + 'project_id' + end + def to_reference(target_project = nil, format: :id) super(project, target_project, format: format) end diff --git a/app/models/repository.rb b/app/models/repository.rb index b6653d1853..fe4b73e9aa 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -178,7 +178,7 @@ class Repository before_remove_branch branch = find_branch(branch_name) - oldrev = branch.try(:target).try(:id) + oldrev = branch.try(:dereferenced_target).try(:id) newrev = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name @@ -294,10 +294,10 @@ class Repository # Rugged seems to throw a `ReferenceError` when given branch_names rather # than SHA-1 hashes number_commits_behind = raw_repository. - count_commits_between(branch.target.sha, root_ref_hash) + count_commits_between(branch.dereferenced_target.sha, root_ref_hash) number_commits_ahead = raw_repository. - count_commits_between(root_ref_hash, branch.target.sha) + count_commits_between(root_ref_hash, branch.dereferenced_target.sha) { behind: number_commits_behind, ahead: number_commits_ahead } end @@ -679,11 +679,11 @@ class Repository branches.sort_by(&:name) when 'updated_desc' branches.sort do |a, b| - commit(b.target).committed_date <=> commit(a.target).committed_date + commit(b.dereferenced_target).committed_date <=> commit(a.dereferenced_target).committed_date end when 'updated_asc' branches.sort do |a, b| - commit(a.target).committed_date <=> commit(b.target).committed_date + commit(a.dereferenced_target).committed_date <=> commit(b.dereferenced_target).committed_date end else branches @@ -858,7 +858,7 @@ class Repository branch = find_branch(ref) if branch - last_commit = branch.target + last_commit = branch.dereferenced_target index.read_tree(last_commit.raw_commit.tree) parents = [last_commit.sha] end @@ -945,7 +945,7 @@ class Repository end def revert(user, commit, base_branch, revert_tree_id = nil) - source_sha = find_branch(base_branch).target.sha + source_sha = find_branch(base_branch).dereferenced_target.sha revert_tree_id ||= check_revert_content(commit, base_branch) return false unless revert_tree_id @@ -962,7 +962,7 @@ class Repository end def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil) - source_sha = find_branch(base_branch).target.sha + source_sha = find_branch(base_branch).dereferenced_target.sha cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch) return false unless cherry_pick_tree_id @@ -991,7 +991,7 @@ class Repository end def check_revert_content(commit, base_branch) - source_sha = find_branch(base_branch).target.sha + source_sha = find_branch(base_branch).dereferenced_target.sha args = [commit.id, source_sha] args << { mainline: 1 } if commit.merge_commit? @@ -1005,7 +1005,7 @@ class Repository end def check_cherry_pick_content(commit, base_branch) - source_sha = find_branch(base_branch).target.sha + source_sha = find_branch(base_branch).dereferenced_target.sha args = [commit.id, source_sha] args << 1 if commit.merge_commit? @@ -1078,7 +1078,7 @@ class Repository if rugged.lookup(newrev).parent_ids.empty? || target_branch.nil? oldrev = Gitlab::Git::BLANK_SHA else - oldrev = rugged.merge_base(newrev, target_branch.target.sha) + oldrev = rugged.merge_base(newrev, target_branch.dereferenced_target.sha) end GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do @@ -1138,7 +1138,7 @@ class Repository end def tags_sorted_by_committed_date - tags.sort_by { |tag| tag.target.committed_date } + tags.sort_by { |tag| tag.dereferenced_target.committed_date } end def keep_around_ref_name(sha) diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index bd1811a3c5..c7114b3eae 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -1,4 +1,8 @@ class IssuePolicy < IssuablePolicy + # This class duplicates the same check of Issue#readable_by? for performance reasons + # Make sure to sync this class checks with issue.rb to avoid security problems. + # Check commit 002ad215818450d2cbbc5fa065850a953dc7ada8 for more information. + def issue @subject end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index fbb3d4507d..1ee31023e2 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -2,11 +2,11 @@ class ProjectPolicy < BasePolicy def rules team_access!(user) - owner = user.admin? || - project.owner == user || + owner = project.owner == user || (project.group && project.group.has_owner?(user)) - owner_access! if owner + owner_access! if user.admin? || owner + team_member_owner_access! if owner if project.public? || (project.internal? && !user.external?) guest_access! @@ -16,7 +16,7 @@ class ProjectPolicy < BasePolicy can! :read_build if project.public_builds? if project.request_access_enabled && - !(owner || project.team.member?(user) || project_group_member?(user)) + !(owner || user.admin? || project.team.member?(user) || project_group_member?(user)) can! :request_access end end @@ -135,6 +135,10 @@ class ProjectPolicy < BasePolicy can! :destroy_issue end + def team_member_owner_access! + team_member_reporter_access! + end + # Push abilities on the users team role def team_access!(user) access = project.team.max_member_access(user.id) diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 8ea88da8a5..c00c5aebf5 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -9,8 +9,8 @@ module Auth return error('UNAVAILABLE', status: 404, message: 'registry not enabled') unless registry.enabled - unless current_user || project - return error('DENIED', status: 403, message: 'access forbidden') unless scope + unless scope || current_user || project + return error('DENIED', status: 403, message: 'access forbidden') end { token: authorized_token(scope).encoded } @@ -76,7 +76,7 @@ module Auth case requested_action when 'pull' - requested_project.public? || build_can_pull?(requested_project) || user_can_pull?(requested_project) + build_can_pull?(requested_project) || user_can_pull?(requested_project) when 'push' build_can_push?(requested_project) || user_can_push?(requested_project) else @@ -92,23 +92,23 @@ module Auth # Build can: # 1. pull from its own project (for ex. a build) # 2. read images from dependent projects if creator of build is a team member - @authentication_abilities.include?(:build_read_container_image) && + has_authentication_ability?(:build_read_container_image) && (requested_project == project || can?(current_user, :build_read_container_image, requested_project)) end def user_can_pull?(requested_project) - @authentication_abilities.include?(:read_container_image) && + has_authentication_ability?(:read_container_image) && can?(current_user, :read_container_image, requested_project) end def build_can_push?(requested_project) # Build can push only to the project from which it originates - @authentication_abilities.include?(:build_create_container_image) && + has_authentication_ability?(:build_create_container_image) && requested_project == project end def user_can_push?(requested_project) - @authentication_abilities.include?(:create_container_image) && + has_authentication_ability?(:create_container_image) && can?(current_user, :create_container_image, requested_project) end @@ -118,5 +118,9 @@ module Auth http_status: status } end + + def has_authentication_ability?(capability) + (@authentication_abilities || []).include?(capability) + end end end diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 918eddaa53..3e5dd4ebb8 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -42,7 +42,7 @@ class DeleteBranchService < BaseService Gitlab::DataBuilder::Push.build( project, current_user, - branch.target.sha, + branch.dereferenced_target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", []) diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb index d0cb151a01..d824406cb4 100644 --- a/app/services/delete_tag_service.rb +++ b/app/services/delete_tag_service.rb @@ -36,7 +36,7 @@ class DeleteTagService < BaseService Gitlab::DataBuilder::Push.build( project, current_user, - tag.target.sha, + tag.dereferenced_target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", []) diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index e6002b03b9..20a4445bdd 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -27,8 +27,8 @@ class GitTagPushService < BaseService tag_name = Gitlab::Git.ref_name(params[:ref]) tag = project.repository.find_tag(tag_name) - if tag && tag.object_sha == params[:newrev] - commit = project.commit(tag.target) + if tag && tag.target == params[:newrev] + commit = project.commit(tag.dereferenced_target) commits = [commit].compact message = tag.message end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index c4c68cd789..28003e5f50 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -353,9 +353,9 @@ %fieldset %legend Repository Storage .form-group - = f.label :repository_storage, 'Storage path for new projects', class: 'control-label col-sm-2' + = f.label :repository_storages, 'Storage paths for new projects', class: 'control-label col-sm-2' .col-sm-10 - = f.select :repository_storage, repository_storage_options_for_select, {}, class: 'form-control' + = f.select :repository_storages, repository_storages_options_for_select, {include_hidden: false}, multiple: true, class: 'form-control' .help-block Manage repository storage paths. Learn more in the = succeed "." do diff --git a/app/views/devise/shared/_tabs_ldap.html.haml b/app/views/devise/shared/_tabs_ldap.html.haml index 1e957f0935..aec1b31ce6 100644 --- a/app/views/devise/shared/_tabs_ldap.html.haml +++ b/app/views/devise/shared/_tabs_ldap.html.haml @@ -8,3 +8,6 @@ - if signin_enabled? %li = link_to 'Standard', '#ldap-standard', 'data-toggle' => 'tab' + - if signin_enabled? && signup_enabled? + %li + = link_to 'Register', '#register-pane', 'data-toggle' => 'tab' diff --git a/app/views/groups/labels/index.html.haml b/app/views/groups/labels/index.html.haml index 70783a6340..45325d6bc4 100644 --- a/app/views/groups/labels/index.html.haml +++ b/app/views/groups/labels/index.html.haml @@ -13,7 +13,7 @@ .other-labels - if @labels.present? %ul.content-list.manage-labels-list.js-other-labels - = render partial: 'shared/label', collection: @labels, as: :label + = render partial: 'shared/label', subject: @group, collection: @labels, as: :label = paginate @labels, theme: 'gitlab' - else .nothing-here-block diff --git a/app/views/projects/_last_commit.html.haml b/app/views/projects/_last_commit.html.haml index 630ae7d614..8e23d51b22 100644 --- a/app/views/projects/_last_commit.html.haml +++ b/app/views/projects/_last_commit.html.haml @@ -1,7 +1,9 @@ -- if commit.status - = link_to builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{commit.status}" do - = ci_icon_for_status(commit.status) - = ci_label_for_status(commit.status) +- ref = local_assigns.fetch(:ref) +- status = commit.status(ref) +- if status + = link_to builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{status}" do + = ci_icon_for_status(status) + = ci_label_for_status(status) = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit_short_id" = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit), class: "commit-row-message" diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index 3ffc3fcb7a..149ee7c59d 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -20,7 +20,7 @@ %ul.blob-commit-info.hidden-xs - blob_commit = @repository.last_commit_for_path(@commit.id, blob.path) - = render blob_commit, project: @project + = render blob_commit, project: @project, ref: @ref %div#blob-content-holder.blob-content-holder %article.file-holder diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 4480b2f22c..ed30ca7fbd 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -1,4 +1,4 @@ -- commit = @repository.commit(branch.target) +- commit = @repository.commit(branch.dereferenced_target) - bar_graph_width_factor = @max_commits > 0 ? 100.0/@max_commits : 0 - diverging_commit_counts = @repository.diverging_commit_counts(branch) - number_commits_behind = diverging_commit_counts[:behind] diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index fb48aef055..34855c5417 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -1,3 +1,4 @@ +- ref = local_assigns.fetch(:ref) - if @note_counts - note_count = @note_counts.fetch(commit.id, 0) - else @@ -5,7 +6,7 @@ - note_count = notes.user.count - cache_key = [project.path_with_namespace, commit.id, current_application_settings, note_count] -- cache_key.push(commit.status) if commit.status +- cache_key.push(commit.status(ref)) if commit.status(ref) = cache(cache_key, expires_in: 1.day) do %li.commit.js-toggle-container{ id: "commit-#{commit.short_id}" } @@ -18,15 +19,15 @@ %span.commit-row-message.visible-xs-inline · = commit.short_id - - if commit.status + - if commit.status(ref) .visible-xs-inline - = render_commit_status(commit) + = render_commit_status(commit, ref: ref) - if commit.description? %a.text-expander.hidden-xs.js-toggle-button ... .commit-actions.hidden-xs - - if commit.status - = render_commit_status(commit) + - if commit.status(ref) + = render_commit_status(commit, ref: ref) = clipboard_button(clipboard_text: commit.id) = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit-short-id btn btn-transparent" = link_to_browse_code(project, commit) diff --git a/app/views/projects/commits/_commit_list.html.haml b/app/views/projects/commits/_commit_list.html.haml index 46e4de4004..ce416caa49 100644 --- a/app/views/projects/commits/_commit_list.html.haml +++ b/app/views/projects/commits/_commit_list.html.haml @@ -11,4 +11,4 @@ %li.warning-row.unstyled #{number_with_delimiter(hidden)} additional commits have been omitted to prevent performance issues. - else - %ul.content-list= render commits, project: @project + %ul.content-list= render commits, project: @project, ref: @ref diff --git a/app/views/projects/commits/_commits.html.haml b/app/views/projects/commits/_commits.html.haml index dd12eae8f7..48756c6894 100644 --- a/app/views/projects/commits/_commits.html.haml +++ b/app/views/projects/commits/_commits.html.haml @@ -1,13 +1,11 @@ -- unless defined?(project) - - project = @project - +- ref = local_assigns.fetch(:ref) - commits, hidden = limited_commits(@commits) - commits.chunk { |c| c.committed_date.in_time_zone.to_date }.each do |day, commits| %li.commit-header= "#{day.strftime('%d %b, %Y')} #{pluralize(commits.count, 'commit')}" %li.commits-row %ul.list-unstyled.commit-list - = render commits, project: project + = render commits, project: project, ref: ref - if hidden > 0 %li.alert.alert-warning diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml index 876c800262..9628cbd163 100644 --- a/app/views/projects/commits/show.html.haml +++ b/app/views/projects/commits/show.html.haml @@ -35,7 +35,7 @@ %div{id: dom_id(@project)} %ol#commits-list.list-unstyled.content_list - = render "commits", project: @project + = render 'commits', project: @project, ref: @ref = spinner :javascript diff --git a/app/views/projects/issues/_related_branches.html.haml b/app/views/projects/issues/_related_branches.html.haml index 44683c8bcd..1892ebb512 100644 --- a/app/views/projects/issues/_related_branches.html.haml +++ b/app/views/projects/issues/_related_branches.html.haml @@ -4,7 +4,7 @@ %ul.unstyled-list.related-merge-requests - @related_branches.each do |branch| %li - - target = @project.repository.find_branch(branch).target + - target = @project.repository.find_branch(branch).dereferenced_target - pipeline = @project.pipeline_for(branch, target.sha) if target - if pipeline %span.related-branch-ci-status diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index f135bf6f6b..05a8475dcd 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -22,14 +22,14 @@ %ul.content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_namespace_project_labels_path(@project.namespace, @project) } %p.empty-message{ class: ('hidden' unless @prioritized_labels.empty?) } No prioritized labels yet - if @prioritized_labels.present? - = render partial: 'shared/label', collection: @prioritized_labels, as: :label + = render partial: 'shared/label', subject: @project, collection: @prioritized_labels, as: :label .other-labels - if can?(current_user, :admin_label, @project) %h5{ class: ('hide' if hide) } Other Labels %ul.content-list.manage-labels-list.js-other-labels - if @labels.present? - = render partial: 'shared/label', collection: @labels, as: :label + = render partial: 'shared/label', subject: @project, collection: @labels, as: :label = paginate @labels, theme: 'gitlab' - if @labels.blank? .nothing-here-block diff --git a/app/views/projects/merge_requests/branch_from.html.haml b/app/views/projects/merge_requests/branch_from.html.haml index 4f90dde6fa..3837c4b388 100644 --- a/app/views/projects/merge_requests/branch_from.html.haml +++ b/app/views/projects/merge_requests/branch_from.html.haml @@ -1 +1,2 @@ -= commit_to_html(@commit, @source_project, false) +- if @commit + = commit_to_html(@commit, @ref, @source_project) diff --git a/app/views/projects/merge_requests/branch_to.html.haml b/app/views/projects/merge_requests/branch_to.html.haml index 67a7a6bcec..d69b71790a 100644 --- a/app/views/projects/merge_requests/branch_to.html.haml +++ b/app/views/projects/merge_requests/branch_to.html.haml @@ -1 +1,2 @@ -= commit_to_html(@commit, @target_project, false) +- if @commit + = commit_to_html(@commit, @ref, @target_project) diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml index 0b05785430..a0e12fb3f3 100644 --- a/app/views/projects/merge_requests/show/_commits.html.haml +++ b/app/views/projects/merge_requests/show/_commits.html.haml @@ -3,4 +3,4 @@ Most recent commits displayed first %ol#commits-list.list-unstyled - = render "projects/commits/commits", project: @merge_request.project + = render "projects/commits/commits", project: @merge_request.source_project, ref: @merge_request.source_branch diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index 399ccf15b7..9436c64ee2 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -90,7 +90,8 @@ = f.label :visibility_level, class: 'label-light' do Visibility Level = link_to "(?)", help_page_path("public_access/public_access") - = render('shared/visibility_radios', model_method: :visibility_level, form: f, selected_level: @project.visibility_level, form_model: @project) + = render 'shared/visibility_level', f: f, visibility_level: default_project_visibility, can_change_visibility_level: true, form_model: @project + = f.submit 'Create project', class: "btn btn-create project-submit", tabindex: 4 = link_to 'Cancel', dashboard_projects_path, class: 'btn btn-cancel' diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index ba16c64146..69a3bc5f04 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -79,7 +79,7 @@ = render 'shared/notifications/button', notification_setting: @notification_setting - if @repository.commit .project-last-commit{ class: container_class } - = render 'projects/last_commit', commit: @repository.commit, project: @project + = render 'projects/last_commit', commit: @repository.commit, ref: current_ref, project: @project %div{ class: container_class } - if @project.archived? diff --git a/app/views/projects/tags/_tag.html.haml b/app/views/projects/tags/_tag.html.haml index 05fccb4f97..c42641afea 100644 --- a/app/views/projects/tags/_tag.html.haml +++ b/app/views/projects/tags/_tag.html.haml @@ -1,4 +1,4 @@ -- commit = @repository.commit(tag.target) +- commit = @repository.commit(tag.dereferenced_target) - release = @releases.find { |release| release.tag == tag.name } %li %div diff --git a/app/views/search/results/_commit.html.haml b/app/views/search/results/_commit.html.haml index 5b2d83d6b9..f34eaf8902 100644 --- a/app/views/search/results/_commit.html.haml +++ b/app/views/search/results/_commit.html.haml @@ -1 +1 @@ -= render 'projects/commits/commit', project: @project, commit: commit += render 'projects/commits/commit', project: @project, commit: commit, ref: nil diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml index 40c8d2af22..6ccdef0df4 100644 --- a/app/views/shared/_label.html.haml +++ b/app/views/shared/_label.html.haml @@ -1,6 +1,7 @@ - label_css_id = dom_id(label) -- open_issues_count = label.open_issues_count(current_user, @project) -- open_merge_requests_count = label.open_merge_requests_count(current_user, @project) +- open_issues_count = label.open_issues_count(current_user) +- open_merge_requests_count = label.open_merge_requests_count(current_user) +- subject = local_assigns[:subject] %li{id: label_css_id, data: { id: label.id } } = render "shared/label_row", label: label @@ -12,10 +13,10 @@ .dropdown-menu.dropdown-menu-align-right %ul %li - = link_to_label(label, subject: @project, type: :merge_request) do + = link_to_label(label, subject: subject, type: :merge_request) do = pluralize open_merge_requests_count, 'merge request' %li - = link_to_label(label, subject: @project) do + = link_to_label(label, subject: subject) do = pluralize open_issues_count, 'open issue' - if current_user %li.label-subscription{ data: toggle_subscription_data(label) } @@ -28,9 +29,9 @@ = link_to 'Delete', destroy_label_path(label), title: 'Delete', method: :delete, remote: true, data: {confirm: 'Remove this label? Are you sure?'} .pull-right.hidden-xs.hidden-sm.hidden-md - = link_to_label(label, subject: @project, type: :merge_request, css_class: 'btn btn-transparent btn-action') do + = link_to_label(label, subject: subject, type: :merge_request, css_class: 'btn btn-transparent btn-action') do = pluralize open_merge_requests_count, 'merge request' - = link_to_label(label, subject: @project, css_class: 'btn btn-transparent btn-action') do + = link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action') do = pluralize open_issues_count, 'open issue' - if current_user diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index d410755cad..ce91b5966a 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -142,6 +142,7 @@ .col-sm-10.col-sm-offset-2 .checkbox = label_tag 'merge_request[force_remove_source_branch]' do + = hidden_field_tag 'merge_request[force_remove_source_branch]', '0', id: nil = check_box_tag 'merge_request[force_remove_source_branch]', '1', @merge_request.force_remove_source_branch? Remove source branch when merge request is accepted. diff --git a/app/views/shared/issuable/_milestone_dropdown.html.haml b/app/views/shared/issuable/_milestone_dropdown.html.haml index f27a9002ec..40fe53e6a8 100644 --- a/app/views/shared/issuable/_milestone_dropdown.html.haml +++ b/app/views/shared/issuable/_milestone_dropdown.html.haml @@ -1,10 +1,10 @@ - project = @target_project || @project - extra_class = extra_class || '' - show_menu_above = show_menu_above || false -- selected_text = selected.try(:title) +- selected_text = selected.try(:title) || params[:milestone_title] - dropdown_title = local_assigns.fetch(:dropdown_title, "Filter by milestone") - if selected.present? - = hidden_field_tag(name, name == :milestone_title ? selected.title : selected.id) + = hidden_field_tag(name, name == :milestone_title ? selected_text : selected.id) = dropdown_tag(milestone_dropdown_label(selected_text), options: { title: dropdown_title, toggle_class: "js-milestone-select js-filter-submit #{extra_class}", filter: true, dropdown_class: "dropdown-menu-selectable dropdown-menu-milestone", placeholder: "Search milestones", footer_content: project.present?, data: { show_no: true, show_menu_above: show_menu_above, show_any: show_any, show_upcoming: show_upcoming, field_name: name, selected: selected.try(:title), project_id: project.try(:id), milestones: milestones_filter_dropdown_path, default_label: "Milestone" } }) do - if project diff --git a/db/migrate/20161103171205_rename_repository_storage_column.rb b/db/migrate/20161103171205_rename_repository_storage_column.rb new file mode 100644 index 0000000000..9328057393 --- /dev/null +++ b/db/migrate/20161103171205_rename_repository_storage_column.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RenameRepositoryStorageColumn < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = true + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + DOWNTIME_REASON = 'Renaming the application_settings.repository_storage column' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + rename_column :application_settings, :repository_storage, :repository_storages + end +end diff --git a/db/schema.rb b/db/schema.rb index 02282b0f66..a5f5f812f0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161024042317) do +ActiveRecord::Schema.define(version: 20161103171205) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -88,7 +88,7 @@ ActiveRecord::Schema.define(version: 20161024042317) do t.integer "container_registry_token_expire_delay", default: 5 t.text "after_sign_up_text" t.boolean "user_default_external", default: false, null: false - t.string "repository_storage", default: "default" + t.string "repository_storages", default: "default" t.string "enabled_git_access_protocol" t.boolean "domain_blacklist_enabled", default: false t.text "domain_blacklist" diff --git a/doc/administration/img/repository_storages_admin_ui.png b/doc/administration/img/repository_storages_admin_ui.png index 599350bc09..6481baca1a 100644 Binary files a/doc/administration/img/repository_storages_admin_ui.png and b/doc/administration/img/repository_storages_admin_ui.png differ diff --git a/doc/administration/repository_storages.md b/doc/administration/repository_storages.md index 55b054fc1a..ab70557b69 100644 --- a/doc/administration/repository_storages.md +++ b/doc/administration/repository_storages.md @@ -91,6 +91,9 @@ be stored via the **Application Settings** in the Admin area. ![Choose repository storage path in Admin area](img/repository_storages_admin_ui.png) +Beginning with GitLab 8.13.4, multiple paths can be chosen. New projects will be +randomly placed on one of the selected paths. + [ce-4578]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4578 [restart gitlab]: restart_gitlab.md#installations-from-source [reconfigure gitlab]: restart_gitlab.md#omnibus-gitlab-reconfigure diff --git a/doc/api/settings.md b/doc/api/settings.md index f7ad3b4cc8..218546aafe 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -42,6 +42,7 @@ Example response: "sign_in_text" : null, "container_registry_token_expire_delay": 5, "repository_storage": "default", + "repository_storages": ["default"], "koding_enabled": false, "koding_url": null } @@ -73,7 +74,8 @@ PUT /application/settings | `user_oauth_applications` | boolean | no | Allow users to register any application to use GitLab as an OAuth provider | | `after_sign_out_path` | string | no | Where to redirect users after logout | | `container_registry_token_expire_delay` | integer | no | Container Registry token duration in minutes | -| `repository_storage` | string | no | Storage path for new projects. The value should be the name of one of the repository storage paths defined in your gitlab.yml | +| `repository_storages` | array of strings | no | A list of names of enabled storage paths, taken from `gitlab.yml`. New projects will be created in one of these stores, chosen at random. | +| `repository_storage` | string | no | The first entry in `repository_storages`. Deprecated, but retained for compatibility reasons | | `enabled_git_access_protocol` | string | no | Enabled protocols for Git access. Allowed values are: `ssh`, `http`, and `nil` to allow both protocols. | | `koding_enabled` | boolean | no | Enable Koding integration. Default is `false`. | | `koding_url` | string | yes (if `koding_enabled` is `true`) | The Koding instance URL for integration. | diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index 2574c2c047..bbcd26477f 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -66,6 +66,12 @@ producing errors whenever it tries to use the `dummy` column. As a result of the above downtime _is_ required when removing a column, even when using PostgreSQL. +## Renaming Columns + +Renaming columns requires downtime as running GitLab instances will continue +using the old column name until a new version is deployed. This can result +in the instance producing errors, which in turn can impact the user experience. + ## Changing Column Constraints Generally changing column constraints requires checking all rows in the table to diff --git a/lib/api/entities.rb b/lib/api/entities.rb index feaa0c213b..81ce5e807e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -138,7 +138,7 @@ module API expose :name expose :commit do |repo_branch, options| - options[:project].repository.commit(repo_branch.target) + options[:project].repository.commit(repo_branch.dereferenced_target) end expose :protected do |repo_branch, options| @@ -510,6 +510,7 @@ module API expose :after_sign_out_path expose :container_registry_token_expire_delay expose :repository_storage + expose :repository_storages expose :koding_enabled expose :koding_url end @@ -523,7 +524,7 @@ module API expose :name, :message expose :commit do |repo_tag, options| - options[:project].repository.commit(repo_tag.target) + options[:project].repository.commit(repo_tag.dereferenced_target) end expose :release, using: Entities::Release do |repo_tag, options| diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 8025581d3c..3c9d7b1aae 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -1,18 +1,12 @@ module API module Helpers + include Gitlab::Utils + PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN" PRIVATE_TOKEN_PARAM = :private_token SUDO_HEADER = "HTTP_SUDO" SUDO_PARAM = :sudo - def to_boolean(value) - return value if [true, false].include?(value) - return true if value =~ /^(true|t|yes|y|1|on)$/i - return false if value =~ /^(false|f|no|n|0|off)$/i - - nil - end - def private_token params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER] end diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 642e6345b9..40d7f4a515 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -29,8 +29,8 @@ module API required_attributes! [:name, :color] attrs = attributes_for_keys [:name, :color, :description] - label = user_project.find_label(attrs[:name]) + label = available_labels.find_by(title: attrs[:name]) conflict!('Label already exists') if label label = user_project.labels.create(attrs) @@ -54,7 +54,7 @@ module API authorize! :admin_label, user_project required_attributes! [:name] - label = user_project.find_label(params[:name]) + label = user_project.labels.find_by(title: params[:name]) not_found!('Label') unless label label.destroy @@ -75,7 +75,7 @@ module API authorize! :admin_label, user_project required_attributes! [:name] - label = user_project.find_label(params[:name]) + label = user_project.labels.find_by(title: params[:name]) not_found!('Label not found') unless label attrs = attributes_for_keys [:new_name, :color, :description] diff --git a/lib/api/settings.rb b/lib/api/settings.rb index c885fcd7ea..c4cb1c7924 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -17,12 +17,12 @@ module API present current_settings, with: Entities::ApplicationSetting end - # Modify applicaiton settings + # Modify application settings # # Example Request: # PUT /application/settings put "application/settings" do - attributes = current_settings.attributes.keys - ["id"] + attributes = ["repository_storage"] + current_settings.attributes.keys - ["id"] attrs = attributes_for_keys(attributes) if current_settings.update_attributes(attrs) diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb index 799b83b106..80c844baec 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 f5d110e987..d8a855ec1f 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 0fee9d267d..8c54a041cb 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 69d01f8db1..0878b6afba 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 a1264db211..6e7b766957 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 e5d1eb11d7..aa76c64ac5 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 c9a9ca79c0..40451947e6 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 a000ac61e5..d3968d6b22 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 fa71b3c952..63b592137b 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 863f5725d3..7adaffa19c 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/banzai/renderer.rb b/lib/banzai/renderer.rb index ce048a36fa..f31fb6c3f7 100644 --- a/lib/banzai/renderer.rb +++ b/lib/banzai/renderer.rb @@ -46,7 +46,7 @@ module Banzai return html if html.present? html = cacheless_render_field(object, field) - object.update_column(html_field, html) unless object.new_record? || object.destroyed? + update_object(object, html_field, html) unless object.new_record? || object.destroyed? html end @@ -166,5 +166,9 @@ module Banzai return unless cache_key Rails.cache.send(:expanded_key, full_cache_key(cache_key, pipeline_name)) end + + def update_object(object, html_field, html) + object.update_column(html_field, html) + end end end diff --git a/lib/gitlab/contributions_calendar.rb b/lib/gitlab/contributions_calendar.rb index b164f5a2ee..7e3d5647b3 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/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index 4f81863da3..d76aa38f74 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -83,7 +83,7 @@ module Gitlab tag = repository.find_tag(tag_name) if tag - commit = repository.commit(tag.target) + commit = repository.commit(tag.dereferenced_target) commit.try(:sha) end else diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 799794c017..bcbf645599 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/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 0a91d3918d..a8b4dc2a83 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -102,6 +102,8 @@ module Gitlab Gitlab::LDAP::Config.providers.each do |provider| adapter = Gitlab::LDAP::Adapter.new(provider) @ldap_person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) + # The `uid` might actually be a DN. Try it next. + @ldap_person ||= Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter) break if @ldap_person end @ldap_person diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index e59ead5d76..4c395b4266 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -13,5 +13,13 @@ module Gitlab def force_utf8(str) str.force_encoding(Encoding::UTF_8) end + + def to_boolean(value) + return value if [true, false].include?(value) + return true if value =~ /^(true|t|yes|y|1|on)$/i + return false if value =~ /^(false|f|no|n|0|off)$/i + + nil + end end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 940d54f868..029eff6605 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -39,6 +39,17 @@ describe Projects::MergeRequestsController do end end + shared_examples "loads labels" do |action| + it "loads labels into the @labels variable" do + get action, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: merge_request.iid, + format: 'html' + expect(assigns(:labels)).not_to be_nil + end + end + describe "GET show" do shared_examples "export merge as" do |format| it "does generally work" do @@ -51,6 +62,8 @@ describe Projects::MergeRequestsController do expect(response).to be_success end + it_behaves_like "loads labels", :show + it "generates it" do expect_any_instance_of(MergeRequest).to receive(:"to_#{format}") @@ -340,6 +353,8 @@ describe Projects::MergeRequestsController do get :diffs, params.merge(extra_params) end + it_behaves_like "loads labels", :diffs + context 'with default params' do context 'as html' do before { go(format: 'html') } @@ -546,6 +561,8 @@ describe Projects::MergeRequestsController do format: format end + it_behaves_like "loads labels", :commits + context 'as html' do it 'renders the show template' do go @@ -564,6 +581,14 @@ describe Projects::MergeRequestsController do end end + describe 'GET builds' do + it_behaves_like "loads labels", :builds + end + + describe 'GET pipelines' do + it_behaves_like "loads labels", :pipelines + end + describe 'GET conflicts' do let(:json_response) { JSON.parse(response.body) } diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 4065e2defb..2190d66488 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -49,13 +49,17 @@ FactoryGirl.define do end after(:create) do |project, evaluator| + # Builds and MRs can't have higher visibility level than repository access level. + builds_access_level = [evaluator.builds_access_level, evaluator.repository_access_level].min + merge_requests_access_level = [evaluator.merge_requests_access_level, evaluator.repository_access_level].min + project.project_feature. - update_attributes( + update_attributes!( wiki_access_level: evaluator.wiki_access_level, - builds_access_level: evaluator.builds_access_level, + builds_access_level: builds_access_level, snippets_access_level: evaluator.snippets_access_level, issues_access_level: evaluator.issues_access_level, - merge_requests_access_level: evaluator.merge_requests_access_level, + merge_requests_access_level: merge_requests_access_level, repository_access_level: evaluator.repository_access_level ) end diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 5910803df5..cd53a485ef 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -12,11 +12,15 @@ describe 'Commits' do end let!(:pipeline) do - FactoryGirl.create :ci_pipeline, project: project, sha: project.commit.sha + create(:ci_pipeline, + project: project, + ref: project.default_branch, + sha: project.commit.sha, + status: :success) end context 'commit status is Generic Commit Status' do - let!(:status) { FactoryGirl.create :generic_commit_status, pipeline: pipeline } + let!(:status) { create(:generic_commit_status, pipeline: pipeline) } before do project.team << [@user, :reporter] @@ -39,7 +43,7 @@ describe 'Commits' do end context 'commit status is Ci Build' do - let!(:build) { FactoryGirl.create :ci_build, pipeline: pipeline } + let!(:build) { create(:ci_build, pipeline: pipeline) } let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } context 'when logged as developer' do @@ -48,13 +52,22 @@ describe 'Commits' do end describe 'Project commits' do + let!(:pipeline_from_other_branch) do + create(:ci_pipeline, + project: project, + ref: 'fix', + sha: project.commit.sha, + status: :failed) + end + before do visit namespace_project_commits_path(project.namespace, project, :master) end - it 'shows build status' do + it 'shows correct build status from default branch' do page.within("//li[@id='commit-#{pipeline.short_sha}']") do - expect(page).to have_css(".ci-status-link") + expect(page).to have_css('.ci-status-link') + expect(page).to have_css('.ci-status-icon-success') end end end diff --git a/spec/features/groups/issues_spec.rb b/spec/features/groups/issues_spec.rb new file mode 100644 index 0000000000..476eca17a9 --- /dev/null +++ b/spec/features/groups/issues_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' + +feature 'Group issues page', feature: true do + let(:path) { issues_group_path(group) } + let(:issuable) { create(:issue, project: project, title: "this is my created issuable")} + + include_examples 'project features apply to issuables', Issue +end diff --git a/spec/features/groups/merge_requests_spec.rb b/spec/features/groups/merge_requests_spec.rb new file mode 100644 index 0000000000..a2791b5754 --- /dev/null +++ b/spec/features/groups/merge_requests_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' + +feature 'Group merge requests page', feature: true do + let(:path) { merge_requests_group_path(group) } + let(:issuable) { create(:merge_request, source_project: project, target_project: project, title: "this is my created issuable")} + + include_examples 'project features apply to issuables', MergeRequest +end diff --git a/spec/features/issues/filter_by_milestone_spec.rb b/spec/features/issues/filter_by_milestone_spec.rb index 485dc56006..29b3e606c3 100644 --- a/spec/features/issues/filter_by_milestone_spec.rb +++ b/spec/features/issues/filter_by_milestone_spec.rb @@ -11,6 +11,7 @@ feature 'Issue filtering by Milestone', feature: true do visit_issues(project) filter_by_milestone(Milestone::None.title) + expect(page).to have_css('.milestone-filter .dropdown-toggle-text', text: 'No Milestone') expect(page).to have_css('.issue', count: 1) end @@ -22,6 +23,7 @@ feature 'Issue filtering by Milestone', feature: true do visit_issues(project) filter_by_milestone(Milestone::Upcoming.title) + expect(page).to have_css('.milestone-filter .dropdown-toggle-text', text: 'Upcoming') expect(page).to have_css('.issue', count: 0) end @@ -33,6 +35,7 @@ feature 'Issue filtering by Milestone', feature: true do visit_issues(project) filter_by_milestone(Milestone::Upcoming.title) + expect(page).to have_css('.milestone-filter .dropdown-toggle-text', text: 'Upcoming') expect(page).to have_css('.issue', count: 1) end @@ -44,6 +47,7 @@ feature 'Issue filtering by Milestone', feature: true do visit_issues(project) filter_by_milestone(Milestone::Upcoming.title) + expect(page).to have_css('.milestone-filter .dropdown-toggle-text', text: 'Upcoming') expect(page).to have_css('.issue', count: 0) end end @@ -55,6 +59,7 @@ feature 'Issue filtering by Milestone', feature: true do visit_issues(project) filter_by_milestone(milestone.title) + expect(page).to have_css('.milestone-filter .dropdown-toggle-text', text: milestone.title) expect(page).to have_css('.issue', count: 1) end diff --git a/spec/features/issues/new_branch_button_spec.rb b/spec/features/issues/new_branch_button_spec.rb index fb0c470428..3e424b547c 100644 --- a/spec/features/issues/new_branch_button_spec.rb +++ b/spec/features/issues/new_branch_button_spec.rb @@ -22,6 +22,7 @@ feature 'Start new branch from an issue', feature: true do create(:note, :on_issue, :system, project: project, note: "Mentioned in !#{referenced_mr.iid}") end + let(:referenced_mr) do create(:merge_request, :simple, source_project: project, target_project: project, description: "Fixes ##{issue.iid}", author: user) diff --git a/spec/features/merge_requests/edit_mr_spec.rb b/spec/features/merge_requests/edit_mr_spec.rb index c77e719c5d..c46bd8d449 100644 --- a/spec/features/merge_requests/edit_mr_spec.rb +++ b/spec/features/merge_requests/edit_mr_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' feature 'Edit Merge Request', feature: true do let(:user) { create(:user) } let(:project) { create(:project, :public) } - let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) } + let(:merge_request) { create(:merge_request, :simple, source_project: project) } before do project.team << [user, :master] @@ -28,5 +28,17 @@ feature 'Edit Merge Request', feature: true do expect(page).to have_content 'Someone edited the merge request the same time you did' end + + it 'allows to unselect "Remove source branch"' do + merge_request.update(merge_params: { 'force_remove_source_branch' => '1' }) + expect(merge_request.merge_params['force_remove_source_branch']).to be_truthy + + visit edit_namespace_project_merge_request_path(project.namespace, project, merge_request) + uncheck 'Remove source branch when merge request is accepted' + + click_button 'Save changes' + + expect(page).to have_content 'Remove source branch' + end end end diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb index 1d4484a9ed..d25cf7fb35 100644 --- a/spec/features/projects/features_visibility_spec.rb +++ b/spec/features/projects/features_visibility_spec.rb @@ -41,6 +41,22 @@ describe 'Edit Project Settings', feature: true do end end end + + context "pipelines subtabs" do + it "shows builds when enabled" do + visit namespace_project_pipelines_path(project.namespace, project) + + expect(page).to have_selector(".shortcuts-builds") + end + + it "hides builds when disabled" do + allow(Ability).to receive(:allowed?).with(member, :read_builds, project).and_return(false) + + visit namespace_project_pipelines_path(project.namespace, project) + + expect(page).not_to have_selector(".shortcuts-builds") + end + end end describe 'project features visibility pages' do diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb new file mode 100644 index 0000000000..abfc46601f --- /dev/null +++ b/spec/features/projects/new_project_spec.rb @@ -0,0 +1,19 @@ +require "spec_helper" + +feature "New project", feature: true do + context "Visibility level selector" do + let(:user) { create(:admin) } + + before { login_as(user) } + + Gitlab::VisibilityLevel.options.each do |key, level| + it "sets selector to #{key}" do + stub_application_setting(default_project_visibility: level) + + visit new_project_path + + expect(find_field("project_visibility_level_#{level}")).to be_checked + end + end + end +end diff --git a/spec/features/projects/ref_switcher_spec.rb b/spec/features/projects/ref_switcher_spec.rb index b3ba40b35a..472491188c 100644 --- a/spec/features/projects/ref_switcher_spec.rb +++ b/spec/features/projects/ref_switcher_spec.rb @@ -22,8 +22,20 @@ feature 'Ref switcher', feature: true, js: true do input.native.send_keys :down input.native.send_keys :down input.native.send_keys :enter - - expect(page).to have_content 'expand-collapse-files' end + + expect(page).to have_title 'expand-collapse-files' + end + + it "user selects ref with special characters" do + click_button 'master' + wait_for_ajax + + page.within '.project-refs-form' do + page.fill_in 'Search branches and tags', with: "'test'" + click_link "'test'" + end + + expect(page).to have_title "'test'" end end diff --git a/spec/features/projects/wiki/user_views_wiki_in_project_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_in_project_page_spec.rb new file mode 100644 index 0000000000..b4f5f6b3fc --- /dev/null +++ b/spec/features/projects/wiki/user_views_wiki_in_project_page_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe 'Projects > Wiki > User views wiki in project page', feature: true do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + + before do + project.team << [user, :master] + login_as(user) + end + + context 'when repository is disabled for project' do + before do + project.project_feature.update!( + repository_access_level: ProjectFeature::DISABLED, + merge_requests_access_level: ProjectFeature::DISABLED, + builds_access_level: ProjectFeature::DISABLED + ) + end + + context 'when wiki homepage contains a link' do + before do + WikiPages::CreateService.new( + project, + user, + title: 'home', + content: '[some link](other-page)' + ).execute + end + + it 'displays the correct URL for the link' do + visit namespace_project_path(project.namespace, project) + expect(page).to have_link( + 'some link', + href: namespace_project_wiki_path( + project.namespace, + project, + 'other-page' + ) + ) + end + end + end +end diff --git a/spec/finders/branches_finder_spec.rb b/spec/finders/branches_finder_spec.rb index 6fce11de30..db60c01db0 100644 --- a/spec/finders/branches_finder_spec.rb +++ b/spec/finders/branches_finder_spec.rb @@ -21,7 +21,7 @@ describe BranchesFinder do result = branches_finder.execute recently_updated_branch = repository.branches.max do |a, b| - repository.commit(a.target).committed_date <=> repository.commit(b.target).committed_date + repository.commit(a.dereferenced_target).committed_date <=> repository.commit(b.dereferenced_target).committed_date end expect(result.first.name).to eq(recently_updated_branch.name) diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index 10cfb66ec1..9085cc8deb 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -64,6 +64,21 @@ describe LabelsFinder do expect(finder.execute).to eq [group_label_2, project_label_1, group_label_1] end + + context 'as an administrator' do + it 'does not return labels from another project' do + # Purposefully creating a project with _nothing_ associated to it + isolated_project = create(:empty_project) + admin = create(:admin) + + # project_3 has a label associated to it, which we don't want coming + # back when we ask for the isolated project's labels + project_3.team << [admin, :reporter] + finder = described_class.new(admin, project_id: isolated_project.id) + + expect(finder.execute).to be_empty + end + end end context 'filtering by title' do diff --git a/spec/finders/tags_finder_spec.rb b/spec/finders/tags_finder_spec.rb index 2ac810e478..98b42e264d 100644 --- a/spec/finders/tags_finder_spec.rb +++ b/spec/finders/tags_finder_spec.rb @@ -20,7 +20,7 @@ describe TagsFinder do result = tags_finder.execute recently_updated_tag = repository.tags.max do |a, b| - repository.commit(a.target).committed_date <=> repository.commit(b.target).committed_date + repository.commit(a.dereferenced_target).committed_date <=> repository.commit(b.dereferenced_target).committed_date end expect(result.first.name).to eq(recently_updated_tag.name) diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb index dca7f99757..a6d2ea11fc 100644 --- a/spec/lib/banzai/filter/autolink_filter_spec.rb +++ b/spec/lib/banzai/filter/autolink_filter_spec.rb @@ -99,6 +99,28 @@ describe Banzai::Filter::AutolinkFilter, lib: true do expect(doc.at_css('a')['href']).to eq link end + it 'autolinks rdar' do + link = 'rdar://localhost.com/blah' + doc = filter("See #{link}") + + expect(doc.at_css('a').text).to eq link + expect(doc.at_css('a')['href']).to eq link + end + + it 'does not autolink javascript' do + link = 'javascript://alert(document.cookie);' + doc = filter("See #{link}") + + expect(doc.at_css('a')).to be_nil + end + + it 'does not autolink bad URLs' do + link = 'foo://23423:::asdf' + doc = filter("See #{link}") + + expect(doc.to_s).to eq("See #{link}") + end + it 'does not include trailing punctuation' do doc = filter("See #{link}.") expect(doc.at_css('a').text).to eq link diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb index f181125156..0140a91c7b 100644 --- a/spec/lib/banzai/filter/redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/redactor_filter_spec.rb @@ -28,31 +28,39 @@ describe Banzai::Filter::RedactorFilter, lib: true do and_return(parser_class) end - it 'removes unpermitted Project references' do - user = create(:user) - project = create(:empty_project) + context 'valid projects' do + before { allow_any_instance_of(Banzai::ReferenceParser::BaseParser).to receive(:can_read_reference?).and_return(true) } - link = reference_link(project: project.id, reference_type: 'test') - doc = filter(link, current_user: user) + it 'allows permitted Project references' do + user = create(:user) + project = create(:empty_project) + project.team << [user, :master] - expect(doc.css('a').length).to eq 0 + link = reference_link(project: project.id, reference_type: 'test') + doc = filter(link, current_user: user) + + expect(doc.css('a').length).to eq 1 + end end - it 'allows permitted Project references' do - user = create(:user) - project = create(:empty_project) - project.team << [user, :master] + context 'invalid projects' do + before { allow_any_instance_of(Banzai::ReferenceParser::BaseParser).to receive(:can_read_reference?).and_return(false) } - link = reference_link(project: project.id, reference_type: 'test') - doc = filter(link, current_user: user) + it 'removes unpermitted references' do + user = create(:user) + project = create(:empty_project) - expect(doc.css('a').length).to eq 1 - end + link = reference_link(project: project.id, reference_type: 'test') + doc = filter(link, current_user: user) - it 'handles invalid Project references' do - link = reference_link(project: 12345, reference_type: 'test') + expect(doc.css('a').length).to eq 0 + end - expect { filter(link) }.not_to raise_error + it 'handles invalid references' do + link = reference_link(project: 12345, reference_type: 'test') + + expect { filter(link) }.not_to raise_error + end end end diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb index 9095d2b134..aa127f0179 100644 --- a/spec/lib/banzai/reference_parser/base_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb @@ -27,41 +27,12 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do let(:link) { empty_html_link } context 'when the link has a data-project attribute' do - it 'returns the nodes if the attribute value equals the current project ID' do + it 'checks if user can read the resource' do link['data-project'] = project.id.to_s - expect(Ability).not_to receive(:allowed?) - expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) - end + expect(subject).to receive(:can_read_reference?).with(user, project) - it 'returns the nodes if the user can read the project' do - other_project = create(:empty_project, :public) - - link['data-project'] = other_project.id.to_s - - expect(Ability).to receive(:allowed?). - with(user, :read_project, other_project). - and_return(true) - - expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) - end - - it 'returns an empty Array when the attribute value is empty' do - link['data-project'] = '' - - expect(subject.nodes_visible_to_user(user, [link])).to eq([]) - end - - it 'returns an empty Array when the user can not read the project' do - other_project = create(:empty_project, :public) - - link['data-project'] = other_project.id.to_s - - expect(Ability).to receive(:allowed?). - with(user, :read_project, other_project). - and_return(false) - - expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + subject.nodes_visible_to_user(user, [link]) end end diff --git a/spec/lib/banzai/reference_parser/commit_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_parser_spec.rb index 0b76d29fce..412ffa77c3 100644 --- a/spec/lib/banzai/reference_parser/commit_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/commit_parser_spec.rb @@ -8,6 +8,14 @@ describe Banzai::ReferenceParser::CommitParser, lib: true do subject { described_class.new(project, user) } let(:link) { empty_html_link } + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before { link['data-commit'] = 123 } + + it_behaves_like "referenced feature visibility", "repository" + end + end + describe '#referenced_by' do context 'when the link has a data-project attribute' do before do diff --git a/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb index ba982f3854..96e55b0997 100644 --- a/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb @@ -8,6 +8,14 @@ describe Banzai::ReferenceParser::CommitRangeParser, lib: true do subject { described_class.new(project, user) } let(:link) { empty_html_link } + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before { link['data-commit-range'] = '123..456' } + + it_behaves_like "referenced feature visibility", "repository" + end + end + describe '#referenced_by' do context 'when the link has a data-project attribute' do before do diff --git a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb index a6ef8394fe..50a5d1a19b 100644 --- a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb @@ -8,6 +8,14 @@ describe Banzai::ReferenceParser::ExternalIssueParser, lib: true do subject { described_class.new(project, user) } let(:link) { empty_html_link } + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before { link['data-external-issue'] = 123 } + + it_behaves_like "referenced feature visibility", "issues" + end + end + describe '#referenced_by' do context 'when the link has a data-project attribute' do before do diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb index 85cfe728b6..6873b7b85f 100644 --- a/spec/lib/banzai/reference_parser/issue_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb @@ -4,10 +4,10 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do include ReferenceParserHelpers let(:project) { create(:empty_project, :public) } - let(:user) { create(:user) } - let(:issue) { create(:issue, project: project) } - subject { described_class.new(project, user) } - let(:link) { empty_html_link } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:link) { empty_html_link } + subject { described_class.new(project, user) } describe '#nodes_visible_to_user' do context 'when the link has a data-issue attribute' do @@ -15,6 +15,8 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do link['data-issue'] = issue.id.to_s end + it_behaves_like "referenced feature visibility", "issues" + it 'returns the nodes when the user can read the issue' do expect(Ability).to receive(:issues_readable_by_user). with([issue], user). diff --git a/spec/lib/banzai/reference_parser/label_parser_spec.rb b/spec/lib/banzai/reference_parser/label_parser_spec.rb index 77fda47f0e..8c540d35dd 100644 --- a/spec/lib/banzai/reference_parser/label_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/label_parser_spec.rb @@ -9,6 +9,14 @@ describe Banzai::ReferenceParser::LabelParser, lib: true do subject { described_class.new(project, user) } let(:link) { empty_html_link } + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before { link['data-label'] = label.id.to_s } + + it_behaves_like "referenced feature visibility", "issues", "merge_requests" + end + end + describe '#referenced_by' do describe 'when the link has a data-label attribute' do context 'using an existing label ID' do diff --git a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb index cf89ad598e..cb69ca1680 100644 --- a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb @@ -8,6 +8,19 @@ describe Banzai::ReferenceParser::MergeRequestParser, lib: true do subject { described_class.new(merge_request.target_project, user) } let(:link) { empty_html_link } + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + let(:project) { merge_request.target_project } + + before do + project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) + link['data-merge-request'] = merge_request.id.to_s + end + + it_behaves_like "referenced feature visibility", "merge_requests" + end + end + describe '#referenced_by' do describe 'when the link has a data-merge-request attribute' do context 'using an existing merge request ID' do diff --git a/spec/lib/banzai/reference_parser/milestone_parser_spec.rb b/spec/lib/banzai/reference_parser/milestone_parser_spec.rb index 6aa45a22cc..2d4d589ae3 100644 --- a/spec/lib/banzai/reference_parser/milestone_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/milestone_parser_spec.rb @@ -9,6 +9,14 @@ describe Banzai::ReferenceParser::MilestoneParser, lib: true do subject { described_class.new(project, user) } let(:link) { empty_html_link } + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before { link['data-milestone'] = milestone.id.to_s } + + it_behaves_like "referenced feature visibility", "issues", "merge_requests" + end + end + describe '#referenced_by' do describe 'when the link has a data-milestone attribute' do context 'using an existing milestone ID' do diff --git a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb index 59127b7c5d..d217a77580 100644 --- a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb @@ -9,6 +9,14 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do subject { described_class.new(project, user) } let(:link) { empty_html_link } + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before { link['data-snippet'] = snippet.id.to_s } + + it_behaves_like "referenced feature visibility", "snippets" + end + end + describe '#referenced_by' do describe 'when the link has a data-snippet attribute' do context 'using an existing snippet ID' do diff --git a/spec/lib/banzai/reference_parser/user_parser_spec.rb b/spec/lib/banzai/reference_parser/user_parser_spec.rb index 4e7f82a6e0..fafc2cec54 100644 --- a/spec/lib/banzai/reference_parser/user_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/user_parser_spec.rb @@ -103,6 +103,8 @@ describe Banzai::ReferenceParser::UserParser, lib: true do it 'returns the nodes if the attribute value equals the current project ID' do link['data-project'] = project.id.to_s + # Ensure that we dont call for Ability.allowed? + # When project_id in the node is equal to current project ID expect(Ability).not_to receive(:allowed?) expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) diff --git a/spec/lib/gitlab/closing_issue_extractor_spec.rb b/spec/lib/gitlab/closing_issue_extractor_spec.rb index de3f64249a..1bbaca0739 100644 --- a/spec/lib/gitlab/closing_issue_extractor_spec.rb +++ b/spec/lib/gitlab/closing_issue_extractor_spec.rb @@ -257,8 +257,9 @@ describe Gitlab::ClosingIssueExtractor, lib: true do context 'with an external issue tracker reference' do it 'extracts the referenced issue' do jira_project = create(:jira_project, name: 'JIRA_EXT1') + jira_project.team << [jira_project.creator, :master] jira_issue = ExternalIssue.new("#{jira_project.name}-1", project: jira_project) - closing_issue_extractor = described_class.new jira_project + closing_issue_extractor = described_class.new(jira_project, jira_project.creator) message = "Resolve #{jira_issue.to_reference}" expect(closing_issue_extractor.closed_by_message(message)).to eq([jira_issue]) diff --git a/spec/lib/gitlab/contributions_calendar_spec.rb b/spec/lib/gitlab/contributions_calendar_spec.rb new file mode 100644 index 0000000000..01b2a55b63 --- /dev/null +++ b/spec/lib/gitlab/contributions_calendar_spec.rb @@ -0,0 +1,104 @@ +require 'spec_helper' + +describe Gitlab::ContributionsCalendar do + let(:contributor) { create(:user) } + let(:user) { create(:user) } + + let(:private_project) do + create(:empty_project, :private) do |project| + create(:project_member, user: contributor, project: project) + end + end + + let(:public_project) do + create(:empty_project, :public) do |project| + create(:project_member, user: contributor, project: project) + end + end + + let(:feature_project) do + create(:empty_project, :public, issues_access_level: ProjectFeature::PRIVATE) do |project| + create(:project_member, user: contributor, project: project).project + end + end + + let(:today) { Time.now.to_date } + let(:last_week) { today - 7.days } + let(:last_year) { today - 1.year } + + before do + travel_to today + end + + after do + travel_back + end + + def calendar(current_user = nil) + described_class.new(contributor, current_user) + end + + def create_event(project, day) + @targets ||= {} + @targets[project] ||= create(:issue, project: project, author: contributor) + + Event.create!( + project: project, + action: Event::CREATED, + target: @targets[project], + author: contributor, + created_at: day, + ) + end + + describe '#activity_dates' do + it "returns a hash of date => count" do + create_event(public_project, last_week) + create_event(public_project, last_week) + create_event(public_project, today) + + expect(calendar.activity_dates).to eq(last_week => 2, today => 1) + end + + it "only shows private events to authorized users" do + create_event(private_project, today) + create_event(feature_project, today) + + expect(calendar.activity_dates[today]).to eq(0) + expect(calendar(user).activity_dates[today]).to eq(0) + expect(calendar(contributor).activity_dates[today]).to eq(2) + end + end + + describe '#events_by_date' do + it "returns all events for a given date" do + e1 = create_event(public_project, today) + e2 = create_event(public_project, today) + create_event(public_project, last_week) + + expect(calendar.events_by_date(today)).to contain_exactly(e1, e2) + end + + it "only shows private events to authorized users" do + e1 = create_event(public_project, today) + e2 = create_event(private_project, today) + e3 = create_event(feature_project, today) + create_event(public_project, last_week) + + expect(calendar.events_by_date(today)).to contain_exactly(e1) + expect(calendar(contributor).events_by_date(today)).to contain_exactly(e1, e2, e3) + end + end + + describe '#starting_year' do + it "should be the start of last year" do + expect(calendar.starting_year).to eq(last_year.year) + end + end + + describe '#starting_month' do + it "should be the start of this month" do + expect(calendar.starting_month).to eq(today.month) + end + end +end diff --git a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb index f045463c1c..6b3dfebd85 100644 --- a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb +++ b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Gfm::ReferenceRewriter do let(:new_project) { create(:project, name: 'new') } let(:user) { create(:user) } - before { old_project.team << [user, :guest] } + before { old_project.team << [user, :reporter] } describe '#rewrite' do subject do diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index a5aa387f4f..f1d0a19000 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -66,6 +66,7 @@ describe Gitlab::GitAccess, lib: true do context 'pull code' do it { expect(subject.allowed?).to be_falsey } + it { expect(subject.message).to match(/You are not allowed to download code/) } end end @@ -77,6 +78,7 @@ describe Gitlab::GitAccess, lib: true do context 'pull code' do it { expect(subject.allowed?).to be_falsey } + it { expect(subject.message).to match(/Your account has been blocked/) } end end @@ -84,6 +86,29 @@ describe Gitlab::GitAccess, lib: true do context 'pull code' do it { expect(subject.allowed?).to be_falsey } end + + context 'when project is public' do + let(:public_project) { create(:project, :public) } + let(:guest_access) { Gitlab::GitAccess.new(nil, public_project, 'web', authentication_abilities: []) } + subject { guest_access.check('git-upload-pack', '_any') } + + context 'when repository is enabled' do + it 'give access to download code' do + public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::ENABLED) + + expect(subject.allowed?).to be_truthy + end + end + + context 'when repository is disabled' do + it 'does not give access to download code' do + public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED) + + expect(subject.allowed?).to be_falsey + expect(subject.message).to match(/You are not allowed to download code/) + end + end + end end describe 'deploy key permissions' do @@ -122,6 +147,14 @@ describe Gitlab::GitAccess, lib: true do describe 'build authentication_abilities permissions' do let(:authentication_abilities) { build_authentication_abilities } + describe 'owner' do + let(:project) { create(:project, namespace: user.namespace) } + + context 'pull code' do + it { expect(subject).to be_allowed } + end + end + describe 'reporter user' do before { project.team << [user, :reporter] } diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 576cda595b..576aa5c366 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -18,7 +18,7 @@ describe Gitlab::GitAccessWiki, lib: true do project.team << [user, :developer] end - subject { access.push_access_check(changes) } + subject { access.check('git-receive-pack', changes) } it { expect(subject.allowed?).to be_truthy } end diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 78c669e8fa..fc9e1cb430 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -137,11 +137,12 @@ describe Gitlab::OAuth::User, lib: true do allow(ldap_user).to receive(:username) { uid } allow(ldap_user).to receive(:email) { ['johndoe@example.com', 'john2@example.com'] } allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } - allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) end context "and no account for the LDAP user" do it "creates a user with dual LDAP and omniauth identities" do + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) + oauth_user.save expect(gl_user).to be_valid @@ -159,6 +160,8 @@ describe Gitlab::OAuth::User, lib: true do context "and LDAP user has an account already" do let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } it "adds the omniauth identity to the LDAP account" do + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) + oauth_user.save expect(gl_user).to be_valid @@ -172,6 +175,24 @@ describe Gitlab::OAuth::User, lib: true do ]) end end + + context 'when an LDAP person is not found by uid' do + it 'tries to find an LDAP person by DN and adds the omniauth identity to the user' do + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(nil) + allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user) + + oauth_user.save + + identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } + expect(identities_as_hash) + .to match_array( + [ + { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + { provider: 'twitter', extern_uid: uid } + ] + ) + end + end end context "and no corresponding LDAP person" do diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 7b4ccc8391..bf0ab9635f 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Gitlab::ReferenceExtractor, lib: true do let(:project) { create(:project) } + before { project.team << [project.creator, :developer] } + subject { Gitlab::ReferenceExtractor.new(project, project.creator) } it 'accesses valid user objects' do @@ -42,7 +44,6 @@ describe Gitlab::ReferenceExtractor, lib: true do end it 'accesses valid issue objects' do - project.team << [project.creator, :developer] @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb new file mode 100644 index 0000000000..d5d8731087 --- /dev/null +++ b/spec/lib/gitlab/utils_spec.rb @@ -0,0 +1,35 @@ +describe Gitlab::Utils, lib: true do + def to_boolean(value) + described_class.to_boolean(value) + end + + describe '.to_boolean' do + it 'accepts booleans' do + expect(to_boolean(true)).to be(true) + expect(to_boolean(false)).to be(false) + end + + it 'converts a valid string to a boolean' do + expect(to_boolean(true)).to be(true) + expect(to_boolean('true')).to be(true) + expect(to_boolean('YeS')).to be(true) + expect(to_boolean('t')).to be(true) + expect(to_boolean('1')).to be(true) + expect(to_boolean('ON')).to be(true) + + expect(to_boolean('FaLse')).to be(false) + expect(to_boolean('F')).to be(false) + expect(to_boolean('NO')).to be(false) + expect(to_boolean('n')).to be(false) + expect(to_boolean('0')).to be(false) + expect(to_boolean('oFF')).to be(false) + end + + it 'converts an invalid string to nil' do + expect(to_boolean('fals')).to be_nil + expect(to_boolean('yeah')).to be_nil + expect(to_boolean('')).to be_nil + expect(to_boolean(nil)).to be_nil + end + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index cc215d252f..2b76e056f3 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -41,14 +41,62 @@ describe ApplicationSetting, models: true do subject { setting } end - context 'repository storages inclussion' do + # Upgraded databases will have this sort of content + context 'repository_storages is a String, not an Array' do + before { setting.__send__(:raw_write_attribute, :repository_storages, 'default') } + + it { expect(setting.repository_storages_before_type_cast).to eq('default') } + it { expect(setting.repository_storages).to eq(['default']) } + end + + context 'repository storages' do before do - storages = { 'custom' => 'tmp/tests/custom_repositories' } + storages = { + 'custom1' => 'tmp/tests/custom_repositories_1', + 'custom2' => 'tmp/tests/custom_repositories_2', + 'custom3' => 'tmp/tests/custom_repositories_3', + + } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end - it { is_expected.to allow_value('custom').for(:repository_storage) } - it { is_expected.not_to allow_value('alternative').for(:repository_storage) } + describe 'inclusion' do + it { is_expected.to allow_value('custom1').for(:repository_storages) } + it { is_expected.to allow_value(['custom2', 'custom3']).for(:repository_storages) } + it { is_expected.not_to allow_value('alternative').for(:repository_storages) } + it { is_expected.not_to allow_value(['alternative', 'custom1']).for(:repository_storages) } + end + + describe 'presence' do + it { is_expected.not_to allow_value([]).for(:repository_storages) } + it { is_expected.not_to allow_value("").for(:repository_storages) } + it { is_expected.not_to allow_value(nil).for(:repository_storages) } + end + + describe '.pick_repository_storage' do + it 'uses Array#sample to pick a random storage' do + array = double('array', sample: 'random') + expect(setting).to receive(:repository_storages).and_return(array) + + expect(setting.pick_repository_storage).to eq('random') + end + + describe '#repository_storage' do + it 'returns the first storage' do + setting.repository_storages = ['good', 'bad'] + + expect(setting.repository_storage).to eq('good') + end + end + + describe '#repository_storage=' do + it 'overwrites repository_storages' do + setting.repository_storage = 'overwritten' + + expect(setting.repository_storages).to eq(['overwritten']) + end + end + end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 51be3f3613..e3bb3482d6 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -205,12 +205,53 @@ eos end end - describe '#ci_commits' do - # TODO: kamil - end - describe '#status' do - # TODO: kamil + context 'without arguments for compound status' do + shared_examples 'giving the status from pipeline' do + it do + expect(commit.status).to eq(Ci::Pipeline.status) + end + end + + context 'with pipelines' do + let!(:pipeline) do + create(:ci_empty_pipeline, project: project, sha: commit.sha) + end + + it_behaves_like 'giving the status from pipeline' + end + + context 'without pipelines' do + it_behaves_like 'giving the status from pipeline' + end + end + + context 'when a particular ref is specified' do + let!(:pipeline_from_master) do + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + ref: 'master', + status: 'failed') + end + + let!(:pipeline_from_fix) do + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + ref: 'fix', + status: 'success') + end + + it 'gives pipelines from a particular branch' do + expect(commit.status('master')).to eq(pipeline_from_master.status) + expect(commit.status('fix')).to eq(pipeline_from_fix.status) + end + + it 'gives compound status if ref is nil' do + expect(commit.status(nil)).to eq(commit.status) + end + end end describe '#participants' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index a59d30687f..4606bc24b3 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -97,6 +97,11 @@ describe Issue, "Issuable" do end end + describe '.to_ability_name' do + it { expect(Issue.to_ability_name).to eq("issue") } + it { expect(MergeRequest.to_ability_name).to eq("merge_request") } + end + describe "#today?" do it "returns true when created today" do # Avoid timezone differences and just return exactly what we want diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index 5363aea4d2..9041690023 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -22,4 +22,18 @@ describe ProjectFeaturesCompatibility do expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::DISABLED) end end + + it "converts fields from true to ProjectFeature::ENABLED" do + features.each do |feature| + project.update_attribute("#{feature}_enabled".to_sym, true) + expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::ENABLED) + end + end + + it "converts fields from false to ProjectFeature::DISABLED" do + features.each do |feature| + project.update_attribute("#{feature}_enabled".to_sym, false) + expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::DISABLED) + end + end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 733b79079e..ebe46efc5a 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -27,13 +27,14 @@ describe Event, models: true do end describe "Push event" do - let(:project) { create(:project) } + let(:project) { create(:project, :private) } let(:user) { project.owner } let(:event) { create_event(project, user) } it do expect(event.push?).to be_truthy - expect(event.visible_to_user?).to be_truthy + expect(event.visible_to_user?(user)).to be_truthy + expect(event.visible_to_user?(nil)).to be_falsey expect(event.tag?).to be_falsey expect(event.branch_name).to eq("master") expect(event.author).to eq(user) @@ -66,6 +67,7 @@ describe Event, models: true do let(:admin) { create(:admin) } let(:issue) { create(:issue, project: project, author: author, assignee: assignee) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:note_on_commit) { create(:note_on_commit, project: project) } let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) } let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) } let(:event) { Event.new(project: project, target: target, author_id: author.id) } @@ -75,6 +77,32 @@ describe Event, models: true do project.team << [guest, :guest] end + context 'commit note event' do + let(:target) { note_on_commit } + + it do + aggregate_failures do + expect(event.visible_to_user?(non_member)).to eq true + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq true + expect(event.visible_to_user?(admin)).to eq true + end + end + + context 'private project' do + let(:project) { create(:empty_project, :private) } + + it do + aggregate_failures do + expect(event.visible_to_user?(non_member)).to eq false + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq false + expect(event.visible_to_user?(admin)).to eq true + end + end + end + end + context 'issue event' do context 'for non confidential issues' do let(:target) { issue } diff --git a/spec/models/guest_spec.rb b/spec/models/guest_spec.rb new file mode 100644 index 0000000000..d79f929f7a --- /dev/null +++ b/spec/models/guest_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Guest, lib: true do + let(:public_project) { create(:project, :public) } + let(:private_project) { create(:project, :private) } + let(:internal_project) { create(:project, :internal) } + + describe '.can_pull?' do + context 'when project is private' do + it 'does not allow to pull the repo' do + expect(Guest.can?(:download_code, private_project)).to eq(false) + end + end + + context 'when project is internal' do + it 'does not allow to pull the repo' do + expect(Guest.can?(:download_code, internal_project)).to eq(false) + end + end + + context 'when project is public' do + context 'when repository is disabled' do + it 'does not allow to pull the repo' do + public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED) + + expect(Guest.can?(:download_code, public_project)).to eq(false) + end + end + + context 'when repository is accessible only by team members' do + it 'does not allow to pull the repo' do + public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::PRIVATE) + + expect(Guest.can?(:download_code, public_project)).to eq(false) + end + end + + context 'when repository is enabled' do + it 'allows to pull the repo' do + public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::ENABLED) + + expect(Guest.can?(:download_code, public_project)).to eq(true) + end + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 3b8b743af2..75d104584f 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -22,7 +22,7 @@ describe Issue, models: true do it { is_expected.to have_db_index(:deleted_at) } end - describe 'visible_to_user' do + describe '.visible_to_user' do let(:user) { create(:user) } let(:authorized_user) { create(:user) } let(:project) { create(:project, namespace: authorized_user.namespace) } @@ -102,11 +102,17 @@ describe Issue, models: true do it 'returns the merge request to close this issue' do allow(mr).to receive(:closes_issue?).with(issue).and_return(true) - expect(issue.closed_by_merge_requests).to eq([mr]) + expect(issue.closed_by_merge_requests(mr.author)).to eq([mr]) + end + + it "returns an empty array when the merge request is closed already" do + closed_mr + + expect(issue.closed_by_merge_requests(closed_mr.author)).to eq([]) end it "returns an empty array when the current issue is closed already" do - expect(closed_issue.closed_by_merge_requests).to eq([]) + expect(closed_issue.closed_by_merge_requests(closed_issue.author)).to eq([]) end end @@ -212,7 +218,7 @@ describe Issue, models: true do source_project: subject.project, source_branch: "#{subject.iid}-branch" }) merge_request.create_cross_references!(user) - expect(subject.referenced_merge_requests).not_to be_empty + expect(subject.referenced_merge_requests(user)).not_to be_empty expect(subject.related_branches(user)).to eq([subject.to_branch_name]) end @@ -308,23 +314,6 @@ describe Issue, models: true do end describe '#visible_to_user?' do - context 'with a user' do - let(:user) { build(:user) } - let(:issue) { build(:issue) } - - it 'returns true when the issue is readable' do - expect(issue).to receive(:readable_by?).with(user).and_return(true) - - expect(issue.visible_to_user?(user)).to eq(true) - end - - it 'returns false when the issue is not readable' do - expect(issue).to receive(:readable_by?).with(user).and_return(false) - - expect(issue.visible_to_user?(user)).to eq(false) - end - end - context 'without a user' do let(:issue) { build(:issue) } @@ -340,9 +329,40 @@ describe Issue, models: true do expect(issue.visible_to_user?).to eq(false) end end - end - describe '#readable_by?' do + context 'with a user' do + let(:user) { build(:user) } + let(:issue) { build(:issue) } + + it 'returns true when the issue is readable' do + expect(issue).to receive(:readable_by?).with(user).and_return(true) + + expect(issue.visible_to_user?(user)).to eq(true) + end + + it 'returns false when the issue is not readable' do + expect(issue).to receive(:readable_by?).with(user).and_return(false) + + expect(issue.visible_to_user?(user)).to eq(false) + end + + it 'returns false when feature is disabled' do + expect(issue).not_to receive(:readable_by?) + + issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + + expect(issue.visible_to_user?(user)).to eq(false) + end + + it 'returns false when restricted for members' do + expect(issue).not_to receive(:readable_by?) + + issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + + expect(issue.visible_to_user?(user)).to eq(false) + end + end + describe 'with a regular user that is not a team member' do let(:user) { create(:user) } @@ -352,13 +372,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns false for a confidential issue' do issue = build(:issue, project: project, confidential: true) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end end @@ -369,13 +389,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end end @@ -387,13 +407,13 @@ describe Issue, models: true do it 'returns false for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end end end @@ -404,26 +424,28 @@ describe Issue, models: true do it 'returns false for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end context 'when the user is the project owner' do + before { project.team << [user, :master] } + it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end end @@ -441,13 +463,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end @@ -461,13 +483,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end @@ -481,13 +503,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end end @@ -499,13 +521,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end end @@ -517,13 +539,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_publicly_visible + expect(issue).to be_truthy end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end end @@ -533,13 +555,13 @@ describe Issue, models: true do it 'returns false for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end end @@ -549,13 +571,13 @@ describe Issue, models: true do it 'returns false for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f4dda1ee55..0245897938 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -67,11 +67,11 @@ describe Project, models: true do it { is_expected.to have_many(:notification_settings).dependent(:destroy) } it { is_expected.to have_many(:forks).through(:forked_project_links) } - context 'after create' do - it "creates project feature" do + context 'after initialized' do + it "has a project_feature" do project = FactoryGirl.build(:project) - expect { project.save }.to change{ project.project_feature.present? }.from(false).to(true) + expect(project.project_feature.present?).to be_present end end @@ -837,16 +837,19 @@ describe Project, models: true do context 'repository storage by default' do let(:project) { create(:empty_project) } - subject { project.repository_storage } - before do - storages = { 'alternative_storage' => '/some/path' } + storages = { + 'default' => 'tmp/tests/repositories', + 'picked' => 'tmp/tests/repositories', + } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) - stub_application_setting(repository_storage: 'alternative_storage') - allow_any_instance_of(Project).to receive(:ensure_dir_exist).and_return(true) end - it { is_expected.to eq('alternative_storage') } + it 'picks storage from ApplicationSetting' do + expect_any_instance_of(ApplicationSetting).to receive(:pick_repository_storage).and_return('picked') + + expect(project.repository_storage).to eq('picked') + end end context 'shared runners by default' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 187a1bf2d7..19b3e7e470 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -68,8 +68,8 @@ describe Repository, models: true do double_first = double(committed_date: Time.now) double_last = double(committed_date: Time.now - 1.second) - allow(tag_a).to receive(:target).and_return(double_first) - allow(tag_b).to receive(:target).and_return(double_last) + allow(tag_a).to receive(:dereferenced_target).and_return(double_first) + allow(tag_b).to receive(:dereferenced_target).and_return(double_last) allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end @@ -83,8 +83,8 @@ describe Repository, models: true do double_first = double(committed_date: Time.now - 1.second) double_last = double(committed_date: Time.now) - allow(tag_a).to receive(:target).and_return(double_last) - allow(tag_b).to receive(:target).and_return(double_first) + allow(tag_a).to receive(:dereferenced_target).and_return(double_last) + allow(tag_b).to receive(:dereferenced_target).and_return(double_first) allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end @@ -632,9 +632,9 @@ describe Repository, models: true do context "when the branch wasn't empty" do it 'updates the head' do - expect(repository.find_branch('feature').target.id).to eq(old_rev) + expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev) repository.update_branch_with_hooks(user, 'feature') { new_rev } - expect(repository.find_branch('feature').target.id).to eq(new_rev) + expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev) end end end @@ -659,7 +659,7 @@ describe Repository, models: true do context 'when the update would remove commits from the target branch' do it 'raises an exception' do branch = 'master' - old_rev = repository.find_branch(branch).target.sha + old_rev = repository.find_branch(branch).dereferenced_target.sha # The 'master' branch is NOT an ancestor of new_rev. expect(repository.rugged.merge_base(old_rev, new_rev)).not_to eq(old_rev) diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 658e3c13a7..96249a7d8c 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -6,6 +6,7 @@ describe ProjectPolicy, models: true do let(:dev) { create(:user) } let(:master) { create(:user) } let(:owner) { create(:user) } + let(:admin) { create(:admin) } let(:project) { create(:empty_project, :public, namespace: owner.namespace) } let(:guest_permissions) do @@ -152,6 +153,19 @@ describe ProjectPolicy, models: true do context 'owner' do let(:current_user) { owner } + it do + is_expected.to include(*guest_permissions) + is_expected.to include(*reporter_permissions) + is_expected.to include(*team_member_reporter_permissions) + is_expected.to include(*developer_permissions) + is_expected.to include(*master_permissions) + is_expected.to include(*owner_permissions) + end + end + + context 'admin' do + let(:current_user) { admin } + it do is_expected.to include(*guest_permissions) is_expected.to include(*reporter_permissions) diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index f7fe4c1087..01bb9e955e 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -265,36 +265,6 @@ describe API::Helpers, api: true do end end - describe '.to_boolean' do - it 'accepts booleans' do - expect(to_boolean(true)).to be(true) - expect(to_boolean(false)).to be(false) - end - - it 'converts a valid string to a boolean' do - expect(to_boolean(true)).to be(true) - expect(to_boolean('true')).to be(true) - expect(to_boolean('YeS')).to be(true) - expect(to_boolean('t')).to be(true) - expect(to_boolean('1')).to be(true) - expect(to_boolean('ON')).to be(true) - - expect(to_boolean('FaLse')).to be(false) - expect(to_boolean('F')).to be(false) - expect(to_boolean('NO')).to be(false) - expect(to_boolean('n')).to be(false) - expect(to_boolean('0')).to be(false) - expect(to_boolean('oFF')).to be(false) - end - - it 'converts an invalid string to nil' do - expect(to_boolean('fals')).to be_nil - expect(to_boolean('yeah')).to be_nil - expect(to_boolean('')).to be_nil - expect(to_boolean(nil)).to be_nil - end - end - describe '.handle_api_exception' do before do allow_any_instance_of(self.class).to receive(:sentry_enabled?).and_return(true) diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 867bc615b9..5c0f5eabeb 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -82,7 +82,20 @@ describe API::API, api: true do expect(json_response['message']['title']).to eq(['is invalid']) end - it 'returns 409 if label already exists' do + it 'returns 409 if label already exists in group' do + group = create(:group) + group_label = create(:group_label, group: group) + project.update(group: group) + + post api("/projects/#{project.id}/labels", user), + name: group_label.name, + color: '#FFAABB' + + expect(response).to have_http_status(409) + expect(json_response['message']).to eq('Label already exists') + end + + it 'returns 409 if label already exists in project' do post api("/projects/#{project.id}/labels", user), name: 'label1', color: '#FFAABB' diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index f4903d8e0b..096a8ebab7 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -33,6 +33,7 @@ describe API::API, 'Settings', api: true do expect(json_response['default_projects_limit']).to eq(3) expect(json_response['signin_enabled']).to be_falsey expect(json_response['repository_storage']).to eq('custom') + expect(json_response['repository_storages']).to eq(['custom']) expect(json_response['koding_enabled']).to be_truthy expect(json_response['koding_url']).to eq('http://koding.example.com') end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 27f0fd22ae..f1728d61de 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -115,6 +115,38 @@ describe 'Git HTTP requests', lib: true do end.to raise_error(JWT::DecodeError) end end + + context 'when the repo is public' do + context 'but the repo is disabled' do + it 'does not allow to clone the repo' do + project = create(:project, :public, repository_access_level: ProjectFeature::DISABLED) + + download("#{project.path_with_namespace}.git", {}) do |response| + expect(response).to have_http_status(:unauthorized) + end + end + end + + context 'but the repo is enabled' do + it 'allows to clone the repo' do + project = create(:project, :public, repository_access_level: ProjectFeature::ENABLED) + + download("#{project.path_with_namespace}.git", {}) do |response| + expect(response).to have_http_status(:ok) + end + end + end + + context 'but only project members are allowed' do + it 'does not allow to clone the repo' do + project = create(:project, :public, repository_access_level: ProjectFeature::PRIVATE) + + download("#{project.path_with_namespace}.git", {}) do |response| + expect(response).to have_http_status(:unauthorized) + end + end + end + end end context "when the project is private" do diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index f0ef155bd7..a3e7844b2f 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -20,7 +20,7 @@ describe JwtController do end end - context 'when using authorized request' do + context 'when using authenticated request' do context 'using CI token' do let(:build) { create(:ci_build, :running) } let(:project) { build.project } @@ -65,7 +65,7 @@ describe JwtController do let(:access_token) { create(:personal_access_token, user: user) } let(:headers) { { authorization: credentials(user.username, access_token.token) } } - it 'rejects the authorization attempt' do + it 'accepts the authorization attempt' do expect(response).to have_http_status(200) end end @@ -81,6 +81,20 @@ describe JwtController do end end + context 'when using unauthenticated request' do + it 'accepts the authorization attempt' do + get '/jwt/auth', parameters + + expect(response).to have_http_status(200) + end + + it 'allows read access' do + expect(service).to receive(:execute).with(authentication_abilities: Gitlab::Auth.read_authentication_abilities) + + get '/jwt/auth', parameters + end + end + context 'unknown service' do subject! { get '/jwt/auth', service: 'unknown' } diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index dbdf83a0df..9bfc84c742 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -284,7 +284,17 @@ describe 'Git LFS API and storage' do let(:authorization) { authorize_ci_project } shared_examples 'can download LFS only from own projects' do - context 'for own project' do + context 'for owned project' do + let(:project) { create(:empty_project, namespace: user.namespace) } + + let(:update_permissions) do + project.lfs_objects << lfs_object + end + + it_behaves_like 'responds with a file' + end + + context 'for member of project' do let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:update_permissions) do diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index c64df4979b..bb26513103 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -245,6 +245,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do it_behaves_like 'a pullable' end + + context 'when you are owner' do + let(:project) { create(:empty_project, namespace: current_user.namespace) } + + it_behaves_like 'a pullable' + end end context 'for private' do @@ -266,6 +272,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do it_behaves_like 'a pullable' end + + context 'when you are owner' do + let(:project) { create(:empty_project, namespace: current_user.namespace) } + + it_behaves_like 'a pullable' + end end end end @@ -276,13 +288,21 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end context 'disallow for all' do - let(:project) { create(:empty_project, :public) } + context 'when you are member' do + let(:project) { create(:empty_project, :public) } - before do - project.team << [current_user, :developer] + before do + project.team << [current_user, :developer] + end + + it_behaves_like 'an inaccessible' end - it_behaves_like 'an inaccessible' + context 'when you are owner' do + let(:project) { create(:empty_project, :public, namespace: current_user.namespace) } + + it_behaves_like 'an inaccessible' + end end end end diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index a4fcd44882..0879e3ab4c 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -37,65 +37,138 @@ describe GitTagPushService, services: true do end describe "Git Tag Push Data" do - before do - service.execute - @push_data = service.push_data - @tag_name = Gitlab::Git.ref_name(ref) - @tag = project.repository.find_tag(@tag_name) - @commit = project.commit(@tag.target) - end - subject { @push_data } + let(:tag) { project.repository.find_tag(tag_name) } + let(:commit) { tag.dereferenced_target } - it { is_expected.to include(object_kind: 'tag_push') } - it { is_expected.to include(ref: ref) } - it { is_expected.to include(before: oldrev) } - it { is_expected.to include(after: newrev) } - it { is_expected.to include(message: @tag.message) } - it { is_expected.to include(user_id: user.id) } - it { is_expected.to include(user_name: user.name) } - it { is_expected.to include(project_id: project.id) } + context 'annotated tag' do + let(:tag_name) { Gitlab::Git.ref_name(ref) } - context "with repository data" do - subject { @push_data[:repository] } - - it { is_expected.to include(name: project.name) } - it { is_expected.to include(url: project.url_to_repo) } - it { is_expected.to include(description: project.description) } - it { is_expected.to include(homepage: project.web_url) } - end - - context "with commits" do - subject { @push_data[:commits] } - - it { is_expected.to be_an(Array) } - it 'has 1 element' do - expect(subject.size).to eq(1) + before do + service.execute + @push_data = service.push_data end - context "the commit" do - subject { @push_data[:commits].first } + it { is_expected.to include(object_kind: 'tag_push') } + it { is_expected.to include(ref: ref) } + it { is_expected.to include(before: oldrev) } + it { is_expected.to include(after: newrev) } + it { is_expected.to include(message: tag.message) } + it { is_expected.to include(user_id: user.id) } + it { is_expected.to include(user_name: user.name) } + it { is_expected.to include(project_id: project.id) } - it { is_expected.to include(id: @commit.id) } - it { is_expected.to include(message: @commit.safe_message) } - it { is_expected.to include(timestamp: @commit.date.xmlschema) } - it do - is_expected.to include( - url: [ - Gitlab.config.gitlab.url, - project.namespace.to_param, - project.to_param, - 'commit', - @commit.id - ].join('/') - ) + context "with repository data" do + subject { @push_data[:repository] } + + it { is_expected.to include(name: project.name) } + it { is_expected.to include(url: project.url_to_repo) } + it { is_expected.to include(description: project.description) } + it { is_expected.to include(homepage: project.web_url) } + end + + context "with commits" do + subject { @push_data[:commits] } + + it { is_expected.to be_an(Array) } + it 'has 1 element' do + expect(subject.size).to eq(1) end - context "with a author" do - subject { @push_data[:commits].first[:author] } + context "the commit" do + subject { @push_data[:commits].first } - it { is_expected.to include(name: @commit.author_name) } - it { is_expected.to include(email: @commit.author_email) } + it { is_expected.to include(id: commit.id) } + it { is_expected.to include(message: commit.safe_message) } + it { is_expected.to include(timestamp: commit.date.xmlschema) } + it do + is_expected.to include( + url: [ + Gitlab.config.gitlab.url, + project.namespace.to_param, + project.to_param, + 'commit', + commit.id + ].join('/') + ) + end + + context "with a author" do + subject { @push_data[:commits].first[:author] } + + it { is_expected.to include(name: commit.author_name) } + it { is_expected.to include(email: commit.author_email) } + end + end + end + end + + context 'lightweight tag' do + let(:tag_name) { 'light-tag' } + let(:newrev) { '5937ac0a7beb003549fc5fd26fc247adbce4a52e' } + let(:ref) { "refs/tags/light-tag" } + + before do + # Create the lightweight tag + project.repository.raw_repository.rugged.tags.create(tag_name, newrev) + + # Clear tag list cache + project.repository.expire_tags_cache + + service.execute + @push_data = service.push_data + end + + it { is_expected.to include(object_kind: 'tag_push') } + it { is_expected.to include(ref: ref) } + it { is_expected.to include(before: oldrev) } + it { is_expected.to include(after: newrev) } + it { is_expected.to include(message: tag.message) } + it { is_expected.to include(user_id: user.id) } + it { is_expected.to include(user_name: user.name) } + it { is_expected.to include(project_id: project.id) } + + context "with repository data" do + subject { @push_data[:repository] } + + it { is_expected.to include(name: project.name) } + it { is_expected.to include(url: project.url_to_repo) } + it { is_expected.to include(description: project.description) } + it { is_expected.to include(homepage: project.web_url) } + end + + context "with commits" do + subject { @push_data[:commits] } + + it { is_expected.to be_an(Array) } + it 'has 1 element' do + expect(subject.size).to eq(1) + end + + context "the commit" do + subject { @push_data[:commits].first } + + it { is_expected.to include(id: commit.id) } + it { is_expected.to include(message: commit.safe_message) } + it { is_expected.to include(timestamp: commit.date.xmlschema) } + it do + is_expected.to include( + url: [ + Gitlab.config.gitlab.url, + project.namespace.to_param, + project.to_param, + 'commit', + commit.id + ].join('/') + ) + end + + context "with a author" do + subject { @push_data[:commits].first[:author] } + + it { is_expected.to include(name: commit.author_name) } + it { is_expected.to include(email: commit.author_email) } + end end end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 3ea1273abc..876bfaf085 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -69,7 +69,7 @@ describe Projects::CreateService, services: true do context 'wiki_enabled false does not create wiki repository directory' do before do - @opts.merge!( { project_feature_attributes: { wiki_access_level: ProjectFeature::DISABLED } }) + @opts.merge!(wiki_enabled: false) @project = create_project(@user, @opts) @path = ProjectWiki.new(@project, @user).send(:path_to_repo) end diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb index 62a5b46d47..75c95d7095 100644 --- a/spec/support/cycle_analytics_helpers.rb +++ b/spec/support/cycle_analytics_helpers.rb @@ -49,7 +49,8 @@ module CycleAnalyticsHelpers end def merge_merge_requests_closing_issue(issue) - merge_requests = issue.closed_by_merge_requests + merge_requests = issue.closed_by_merge_requests(user) + merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user).execute(merge_request) } end diff --git a/spec/support/project_features_apply_to_issuables_shared_examples.rb b/spec/support/project_features_apply_to_issuables_shared_examples.rb new file mode 100644 index 0000000000..4621d17549 --- /dev/null +++ b/spec/support/project_features_apply_to_issuables_shared_examples.rb @@ -0,0 +1,56 @@ +shared_examples 'project features apply to issuables' do |klass| + let(:described_class) { klass } + + let(:group) { create(:group) } + let(:user_in_group) { create(:group_member, :developer, user: create(:user), group: group ).user } + let(:user_outside_group) { create(:user) } + + let(:project) { create(:empty_project, :public, project_args) } + + def project_args + feature = "#{described_class.model_name.plural}_access_level".to_sym + + args = { group: group } + args[feature] = access_level + + args + end + + before do + _ = issuable + login_as(user) + visit path + end + + context 'public access level' do + let(:access_level) { ProjectFeature::ENABLED } + + context 'group member' do + let(:user) { user_in_group } + + it { expect(page).to have_content(issuable.title) } + end + + context 'non-member' do + let(:user) { user_outside_group } + + it { expect(page).to have_content(issuable.title) } + end + end + + context 'private access level' do + let(:access_level) { ProjectFeature::PRIVATE } + + context 'group member' do + let(:user) { user_in_group } + + it { expect(page).to have_content(issuable.title) } + end + + context 'non-member' do + let(:user) { user_outside_group } + + it { expect(page).not_to have_content(issuable.title) } + end + end +end diff --git a/spec/support/reference_parser_shared_examples.rb b/spec/support/reference_parser_shared_examples.rb new file mode 100644 index 0000000000..8eb74635a6 --- /dev/null +++ b/spec/support/reference_parser_shared_examples.rb @@ -0,0 +1,43 @@ +RSpec.shared_examples "referenced feature visibility" do |*related_features| + let(:feature_fields) do + related_features.map { |feature| (feature + "_access_level").to_sym } + end + + before { link['data-project'] = project.id.to_s } + + context "when feature is disabled" do + it "does not create reference" do + set_features_fields_to(ProjectFeature::DISABLED) + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context "when feature is enabled only for team members" do + before { set_features_fields_to(ProjectFeature::PRIVATE) } + + it "does not create reference for non member" do + non_member = create(:user) + + expect(subject.nodes_visible_to_user(non_member, [link])).to eq([]) + end + + it "creates reference for member" do + project.team << [user, :developer] + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + + context "when feature is enabled" do + # The project is public + it "creates reference" do + set_features_fields_to(ProjectFeature::ENABLED) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + + def set_features_fields_to(visibility_level) + feature_fields.each { |field| project.project_feature.update_attribute(field, visibility_level) } + end +end diff --git a/spec/views/projects/issues/_related_branches.html.haml_spec.rb b/spec/views/projects/issues/_related_branches.html.haml_spec.rb index c8a3d02d8f..889d9a3888 100644 --- a/spec/views/projects/issues/_related_branches.html.haml_spec.rb +++ b/spec/views/projects/issues/_related_branches.html.haml_spec.rb @@ -5,7 +5,7 @@ describe 'projects/issues/_related_branches' do let(:project) { create(:project) } let(:branch) { project.repository.find_branch('feature') } - let!(:pipeline) { create(:ci_pipeline, project: project, sha: branch.target.id, ref: 'feature') } + let!(:pipeline) { create(:ci_pipeline, project: project, sha: branch.dereferenced_target.id, ref: 'feature') } before do assign(:project, project)