summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Jarvis <jarv@gitlab.com>2019-03-05 10:28:59 +0000
committerJohn Jarvis <jarv@gitlab.com>2019-03-05 10:28:59 +0000
commit83cb7482560c04d14fda372fcb4f95d667cd1962 (patch)
treeec36cfddb359467a35276a99b5bc872b1fdfb7e2
parent960101c20bb8ec5a2d1d3f01474676c7ea95adc7 (diff)
parenta24eabb1782785ad0bcf2460c3fff67106598b83 (diff)
downloadgitlab-ce-83cb7482560c04d14fda372fcb4f95d667cd1962.tar.gz
Merge branch 'merge-dev-to-master' into 'master'
Merge dev.gitlab.org master into GitLab.com master Closes #2794, #2814, #2806, #2805, #2798, #2795, #2788, and #2787 See merge request gitlab-org/gitlab-ce!25746
-rw-r--r--CHANGELOG.md27
-rw-r--r--app/assets/javascripts/behaviors/markdown/render_mermaid.js19
-rw-r--r--app/controllers/concerns/milestone_actions.rb2
-rw-r--r--app/controllers/google_api/authorizations_controller.rb32
-rw-r--r--app/controllers/profiles/active_sessions_controller.rb11
-rw-r--r--app/controllers/projects/autocomplete_sources_controller.rb2
-rw-r--r--app/controllers/projects/commit_controller.rb6
-rw-r--r--app/controllers/projects/group_links_controller.rb5
-rw-r--r--app/finders/merge_requests_finder.rb9
-rw-r--r--app/graphql/types/project_type.rb2
-rw-r--r--app/mailers/emails/issues.rb1
-rw-r--r--app/models/active_session.rb6
-rw-r--r--app/models/clusters/platforms/kubernetes.rb2
-rw-r--r--app/models/concerns/issuable.rb11
-rw-r--r--app/models/concerns/milestoneish.rb11
-rw-r--r--app/models/merge_request.rb9
-rw-r--r--app/models/merge_request_diff.rb2
-rw-r--r--app/models/project_services/prometheus_service.rb6
-rw-r--r--app/policies/group_policy.rb1
-rw-r--r--app/policies/project_policy.rb2
-rw-r--r--app/services/issuable_base_service.rb6
-rw-r--r--app/services/issues/build_service.rb4
-rw-r--r--app/services/merge_requests/build_service.rb1
-rw-r--r--app/services/projects/group_links/create_service.rb10
-rw-r--r--app/uploaders/file_mover.rb8
-rw-r--r--app/validators/sha_validator.rb9
-rw-r--r--app/views/notify/issue_moved_email.html.haml11
-rw-r--r--app/views/notify/issue_moved_email.text.erb4
-rw-r--r--app/views/profiles/active_sessions/_active_session.html.haml6
-rw-r--r--app/views/projects/blob/viewers/_dependency_manager.html.haml5
-rw-r--r--changelogs/unreleased/2802-security-add-public-internal-groups-as-members-to-your-project-idor.yml6
-rw-r--r--changelogs/unreleased/51971-milestones-visibility.yml5
-rw-r--r--changelogs/unreleased/57534_filter_impersonated_sessions.yml6
-rw-r--r--changelogs/unreleased/security-2797-milestone-mrs.yml5
-rw-r--r--changelogs/unreleased/security-2798-fix-boards-policy.yml5
-rw-r--r--changelogs/unreleased/security-2799-emails.yml5
-rw-r--r--changelogs/unreleased/security-50334.yml5
-rw-r--r--changelogs/unreleased/security-55468-check-validity-before-querying.yml5
-rw-r--r--changelogs/unreleased/security-56348.yml5
-rw-r--r--changelogs/unreleased/security-commit-private-related-mr.yml5
-rw-r--r--changelogs/unreleased/security-fj-diff-import-file-read-fix.yml5
-rw-r--r--changelogs/unreleased/security-id-restricted-access-to-private-repo.yml5
-rw-r--r--changelogs/unreleased/security-issue_54789_2.yml5
-rw-r--r--changelogs/unreleased/security-kubernetes-google-login-csrf.yml5
-rw-r--r--changelogs/unreleased/security-kubernetes-local-ssrf.yml5
-rw-r--r--changelogs/unreleased/security-mermaid.yml5
-rw-r--r--changelogs/unreleased/security-osw-stop-linking-to-packages.yml5
-rw-r--r--changelogs/unreleased/security-protect-private-repo-information.yml5
-rw-r--r--changelogs/unreleased/security-shared-project-private-group.yml5
-rw-r--r--changelogs/unreleased/security-tags-oracle.yml5
-rw-r--r--config/routes/git_http.rb2
-rw-r--r--doc/user/profile/active_sessions.md8
-rw-r--r--doc/user/profile/img/active_sessions_list.pngbin22266 -> 19360 bytes
-rw-r--r--lib/api/commits.rb10
-rw-r--r--lib/api/entities.rb9
-rw-r--r--lib/api/environments.rb8
-rw-r--r--lib/api/helpers/notes_helpers.rb14
-rw-r--r--lib/api/projects.rb39
-rw-r--r--lib/api/release/links.rb2
-rw-r--r--lib/constraints/project_url_constrainer.rb3
-rw-r--r--lib/gitlab/auth/omniauth_identity_linker_base.rb6
-rw-r--r--lib/gitlab/dependency_linker/base_linker.rb18
-rw-r--r--lib/gitlab/dependency_linker/composer_json_linker.rb4
-rw-r--r--lib/gitlab/dependency_linker/gemfile_linker.rb30
-rw-r--r--lib/gitlab/dependency_linker/gemspec_linker.rb2
-rw-r--r--lib/gitlab/dependency_linker/method_linker.rb10
-rw-r--r--lib/gitlab/dependency_linker/package.rb19
-rw-r--r--lib/gitlab/dependency_linker/package_json_linker.rb21
-rw-r--r--lib/gitlab/dependency_linker/parser/gemfile.rb40
-rw-r--r--lib/gitlab/dependency_linker/podfile_linker.rb11
-rw-r--r--lib/gitlab/dependency_linker/podspec_linker.rb2
-rw-r--r--lib/gitlab/import_export/merge_request_parser.rb11
-rw-r--r--lib/gitlab/kubernetes/kube_client.rb8
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/controllers/dashboard/milestones_controller_spec.rb2
-rw-r--r--spec/controllers/google_api/authorizations_controller_spec.rb60
-rw-r--r--spec/controllers/groups/shared_projects_controller_spec.rb2
-rw-r--r--spec/controllers/omniauth_callbacks_controller_spec.rb2
-rw-r--r--spec/controllers/projects/autocomplete_sources_controller_spec.rb31
-rw-r--r--spec/controllers/projects/group_links_controller_spec.rb37
-rw-r--r--spec/controllers/snippets_controller_spec.rb4
-rw-r--r--spec/features/issues_spec.rb4
-rw-r--r--spec/features/merge_request/user_sees_versions_spec.rb6
-rw-r--r--spec/features/merge_requests/user_lists_merge_requests_spec.rb4
-rw-r--r--spec/features/profiles/active_sessions_spec.rb48
-rw-r--r--spec/features/projects/blobs/blob_show_spec.rb5
-rw-r--r--spec/features/projects/members/invite_group_spec.rb2
-rw-r--r--spec/features/projects/settings/user_manages_group_links_spec.rb1
-rw-r--r--spec/features/security/group/private_access_spec.rb32
-rw-r--r--spec/finders/merge_requests_finder_spec.rb26
-rw-r--r--spec/lib/constraints/project_url_constrainer_spec.rb4
-rw-r--r--spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb4
-rw-r--r--spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb9
-rw-r--r--spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb4
-rw-r--r--spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb18
-rw-r--r--spec/lib/gitlab/dependency_linker/parser/gemfile_spec.rb44
-rw-r--r--spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb5
-rw-r--r--spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb4
-rw-r--r--spec/lib/gitlab/import_export/merge_request_parser_spec.rb16
-rw-r--r--spec/lib/gitlab/kubernetes/kube_client_spec.rb30
-rw-r--r--spec/mailers/notify_spec.rb58
-rw-r--r--spec/models/active_session_spec.rb5
-rw-r--r--spec/models/clusters/platforms/kubernetes_spec.rb16
-rw-r--r--spec/models/concerns/issuable_spec.rb97
-rw-r--r--spec/models/concerns/milestoneish_spec.rb47
-rw-r--r--spec/models/issue/metrics_spec.rb6
-rw-r--r--spec/models/merge_request_diff_spec.rb14
-rw-r--r--spec/models/milestone_spec.rb8
-rw-r--r--spec/models/project_services/prometheus_service_spec.rb61
-rw-r--r--spec/policies/commit_policy_spec.rb55
-rw-r--r--spec/policies/group_policy_spec.rb40
-rw-r--r--spec/policies/note_policy_spec.rb94
-rw-r--r--spec/policies/project_policy_spec.rb20
-rw-r--r--spec/requests/api/commits_spec.rb15
-rw-r--r--spec/requests/api/issues_spec.rb2
-rw-r--r--spec/requests/api/projects_spec.rb71
-rw-r--r--spec/requests/api/release/links_spec.rb16
-rw-r--r--spec/requests/git_http_spec.rb134
-rw-r--r--spec/services/issuable/common_system_notes_service_spec.rb4
-rw-r--r--spec/services/issues/build_service_spec.rb76
-rw-r--r--spec/services/issues/update_service_spec.rb6
-rw-r--r--spec/services/merge_requests/build_service_spec.rb9
-rw-r--r--spec/services/merge_requests/update_service_spec.rb6
-rw-r--r--spec/services/projects/group_links/create_service_spec.rb8
-rw-r--r--spec/support/helpers/file_mover_helpers.rb12
-rw-r--r--spec/support/helpers/login_helpers.rb13
-rw-r--r--spec/support/shared_examples/issuable_shared_examples.rb2
-rw-r--r--spec/support/shared_examples/requests/api/discussions.rb31
-rw-r--r--spec/uploaders/file_mover_spec.rb33
-rw-r--r--spec/validators/sha_validator_spec.rb42
-rw-r--r--spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb2
131 files changed, 1571 insertions, 448 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index feda5e0835b..8a4a6c9ff13 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -486,6 +486,33 @@ entry.
- Update url placeholder for the sentry configuration page. !24338
+## 11.6.10 (2019-02-28)
+
+### Security (21 changes)
+
+- Stop linking to unrecognized package sources. !55518
+- Check snippet attached file to be moved is within designated directory.
+- Fix potential Addressable::URI::InvalidURIError.
+- Do not display impersonated sessions under active sessions and remove ability to revoke session.
+- Display only information visible to current user on the Milestone page.
+- Show only merge requests visible to user on milestone detail page.
+- Disable issue boards API when issues are disabled.
+- Don't show new issue link after move when a user does not have permissions.
+- Fix git clone revealing private repo's presence.
+- Fix blind SSRF in Prometheus integration by checking URL before querying.
+- Check if desired milestone for an issue is available.
+- Don't allow non-members to see private related MRs.
+- Fix arbitrary file read via diffs during import.
+- Display the correct number of MRs a user has access to.
+- Forbid creating discussions for users with restricted access.
+- Do not disclose milestone titles for unauthorized users.
+- Validate session key when authorizing with GCP to create a cluster.
+- Block local URLs for Kubernetes integration.
+- Limit mermaid rendering to 5K characters.
+- Remove the possibility to share a project with a group that a user is not a member of.
+- Fix leaking private repository information in API.
+
+
## 11.6.8 (2019-01-30)
- No changes.
diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js
index 35380ca49fb..798114b4b0b 100644
--- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js
+++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js
@@ -1,4 +1,5 @@
import flash from '~/flash';
+import { sprintf, __ } from '../../locale';
// Renders diagrams and flowcharts from text using Mermaid in any element with the
// `js-render-mermaid` class.
@@ -14,6 +15,9 @@ import flash from '~/flash';
// </pre>
//
+// This is an arbitary number; Can be iterated upon when suitable.
+const MAX_CHAR_LIMIT = 5000;
+
export default function renderMermaid($els) {
if (!$els.length) return;
@@ -34,6 +38,21 @@ export default function renderMermaid($els) {
$els.each((i, el) => {
const source = el.textContent;
+ /**
+ * Restrict the rendering to a certain amount of character to
+ * prevent mermaidjs from hanging up the entire thread and
+ * causing a DoS.
+ */
+ if (source && source.length > MAX_CHAR_LIMIT) {
+ el.textContent = sprintf(
+ __(
+ 'Cannot render the image. Maximum character count (%{charLimit}) has been exceeded.',
+ ),
+ { charLimit: MAX_CHAR_LIMIT },
+ );
+ return;
+ }
+
// Remove any extra spans added by the backend syntax highlighting.
Object.assign(el, { textContent: source });
diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb
index eccbe35577b..c0c0160a827 100644
--- a/app/controllers/concerns/milestone_actions.rb
+++ b/app/controllers/concerns/milestone_actions.rb
@@ -8,7 +8,7 @@ module MilestoneActions
format.html { redirect_to milestone_redirect_path }
format.json do
render json: tabs_json("shared/milestones/_merge_requests_tab", {
- merge_requests: @milestone.sorted_merge_requests, # rubocop:disable Gitlab/ModuleWithInstanceVariables
+ merge_requests: @milestone.sorted_merge_requests(current_user), # rubocop:disable Gitlab/ModuleWithInstanceVariables
show_project_name: true
})
end
diff --git a/app/controllers/google_api/authorizations_controller.rb b/app/controllers/google_api/authorizations_controller.rb
index dd9f5af61b3..ed0995e7ffd 100644
--- a/app/controllers/google_api/authorizations_controller.rb
+++ b/app/controllers/google_api/authorizations_controller.rb
@@ -2,6 +2,10 @@
module GoogleApi
class AuthorizationsController < ApplicationController
+ include Gitlab::Utils::StrongMemoize
+
+ before_action :validate_session_key!
+
def callback
token, expires_at = GoogleApi::CloudPlatform::Client
.new(nil, callback_google_api_auth_url)
@@ -11,21 +15,27 @@ module GoogleApi
session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] =
expires_at.to_s
- state_redirect_uri = redirect_uri_from_session_key(params[:state])
-
- if state_redirect_uri
- redirect_to state_redirect_uri
- else
- redirect_to root_path
- end
+ redirect_to redirect_uri_from_session
end
private
- def redirect_uri_from_session_key(state)
- key = GoogleApi::CloudPlatform::Client
- .session_key_for_redirect_uri(params[:state])
- session[key] if key
+ def validate_session_key!
+ access_denied! unless redirect_uri_from_session.present?
+ end
+
+ def redirect_uri_from_session
+ strong_memoize(:redirect_uri_from_session) do
+ if params[:state].present?
+ session[session_key_for_redirect_uri(params[:state])]
+ else
+ nil
+ end
+ end
+ end
+
+ def session_key_for_redirect_uri(state)
+ GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(state)
end
end
end
diff --git a/app/controllers/profiles/active_sessions_controller.rb b/app/controllers/profiles/active_sessions_controller.rb
index efe7ede5efa..c473023cacb 100644
--- a/app/controllers/profiles/active_sessions_controller.rb
+++ b/app/controllers/profiles/active_sessions_controller.rb
@@ -2,15 +2,6 @@
class Profiles::ActiveSessionsController < Profiles::ApplicationController
def index
- @sessions = ActiveSession.list(current_user)
- end
-
- def destroy
- ActiveSession.destroy(current_user, params[:id])
-
- respond_to do |format|
- format.html { redirect_to profile_active_sessions_url, status: :found }
- format.js { head :ok }
- end
+ @sessions = ActiveSession.list(current_user).reject(&:is_impersonated)
end
end
diff --git a/app/controllers/projects/autocomplete_sources_controller.rb b/app/controllers/projects/autocomplete_sources_controller.rb
index 9c130af8394..0e3f13045ce 100644
--- a/app/controllers/projects/autocomplete_sources_controller.rb
+++ b/app/controllers/projects/autocomplete_sources_controller.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
class Projects::AutocompleteSourcesController < Projects::ApplicationController
+ before_action :authorize_read_milestone!, only: :milestones
+
def members
render json: ::Projects::ParticipantsService.new(@project, current_user).execute(target)
end
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index b13c0ae3967..939a09d4fd2 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -65,7 +65,11 @@ class Projects::CommitController < Projects::ApplicationController
# rubocop: enable CodeReuse/ActiveRecord
def merge_requests
- @merge_requests = @commit.merge_requests.map do |mr|
+ @merge_requests = MergeRequestsFinder.new(
+ current_user,
+ project_id: @project.id,
+ commit_sha: @commit.sha
+ ).execute.map do |mr|
{ iid: mr.iid, path: merge_request_path(mr), title: mr.title }
end
diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb
index 7c713c19762..bc942ba9288 100644
--- a/app/controllers/projects/group_links_controller.rb
+++ b/app/controllers/projects/group_links_controller.rb
@@ -13,9 +13,10 @@ class Projects::GroupLinksController < Projects::ApplicationController
group = Group.find(params[:link_group_id]) if params[:link_group_id].present?
if group
- return render_404 unless can?(current_user, :read_group, group)
+ result = Projects::GroupLinks::CreateService.new(project, current_user, group_link_create_params).execute(group)
+ return render_404 if result[:http_status] == 404
- Projects::GroupLinks::CreateService.new(project, current_user, group_link_create_params).execute(group)
+ flash[:alert] = result[:message] if result[:http_status] == 409
else
flash[:alert] = 'Please select a group.'
end
diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb
index b645011a3c5..93bee3f1488 100644
--- a/app/finders/merge_requests_finder.rb
+++ b/app/finders/merge_requests_finder.rb
@@ -37,13 +37,20 @@ class MergeRequestsFinder < IssuableFinder
end
def filter_items(_items)
- items = by_source_branch(super)
+ items = by_commit(super)
+ items = by_source_branch(items)
items = by_wip(items)
by_target_branch(items)
end
private
+ def by_commit(items)
+ return items unless params[:commit_sha].presence
+
+ items.by_commit_sha(params[:commit_sha])
+ end
+
def source_branch
@source_branch ||= params[:source_branch].presence
end
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb
index 3ef0cc5020c..b96c2f3afb2 100644
--- a/app/graphql/types/project_type.rb
+++ b/app/graphql/types/project_type.rb
@@ -16,7 +16,6 @@ module Types
field :description, GraphQL::STRING_TYPE, null: true
- field :default_branch, GraphQL::STRING_TYPE, null: true
field :tag_list, GraphQL::STRING_TYPE, null: true
field :ssh_url_to_repo, GraphQL::STRING_TYPE, null: true
@@ -59,7 +58,6 @@ module Types
end
field :import_status, GraphQL::STRING_TYPE, null: true
- field :ci_config_path, GraphQL::STRING_TYPE, null: true
field :only_allow_merge_if_pipeline_succeeds, GraphQL::BOOLEAN_TYPE, null: true
field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true
diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index 654ae211310..d2e334fb856 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -74,6 +74,7 @@ module Emails
@new_issue = new_issue
@new_project = new_issue.project
+ @can_access_project = recipient.can?(:read_project, @new_project)
mail_answer_thread(issue, issue_thread_options(updated_by_user.id, recipient.id, reason))
end
diff --git a/app/models/active_session.rb b/app/models/active_session.rb
index 0d9c6a4a1f0..1e01f1d17e6 100644
--- a/app/models/active_session.rb
+++ b/app/models/active_session.rb
@@ -5,7 +5,8 @@ class ActiveSession
attr_accessor :created_at, :updated_at,
:session_id, :ip_address,
- :browser, :os, :device_name, :device_type
+ :browser, :os, :device_name, :device_type,
+ :is_impersonated
def current?(session)
return false if session_id.nil? || session.id.nil?
@@ -31,7 +32,8 @@ class ActiveSession
device_type: client.device_type,
created_at: user.current_sign_in_at || timestamp,
updated_at: timestamp,
- session_id: session_id
+ session_id: session_id,
+ is_impersonated: request.session[:impersonator_id].present?
)
redis.pipelined do
diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb
index 46d0898014e..814fc591408 100644
--- a/app/models/clusters/platforms/kubernetes.rb
+++ b/app/models/clusters/platforms/kubernetes.rb
@@ -41,7 +41,7 @@ module Clusters
validate :no_namespace, unless: :allow_user_defined_namespace?
# We expect to be `active?` only when enabled and cluster is created (the api_url is assigned)
- validates :api_url, url: true, presence: true
+ validates :api_url, public_url: true, presence: true
validates :token, presence: true
validates :ca_cert, certificate: true, allow_blank: true, if: :ca_cert_changed?
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 670103bc3f3..c7ad182ab82 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -75,6 +75,7 @@ module Issuable
validates :author, presence: true
validates :title, presence: true, length: { maximum: 255 }
+ validate :milestone_is_valid
scope :authored, ->(user) { where(author_id: user) }
scope :recent, -> { reorder(id: :desc) }
@@ -118,6 +119,16 @@ module Issuable
def has_multiple_assignees?
assignees.count > 1
end
+
+ def milestone_available?
+ project_id == milestone&.project_id || project.ancestors_upto.compact.include?(milestone&.group)
+ end
+
+ private
+
+ def milestone_is_valid
+ errors.add(:milestone_id, message: "is invalid") if milestone_id.present? && !milestone_available?
+ end
end
class_methods do
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 055ffe04646..39372c4f68b 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -46,12 +46,19 @@ module Milestoneish
end
end
+ def merge_requests_visible_to_user(user)
+ memoize_per_user(user, :merge_requests_visible_to_user) do
+ MergeRequestsFinder.new(user, {})
+ .execute.where(milestone_id: milestoneish_id)
+ end
+ end
+
def sorted_issues(user)
issues_visible_to_user(user).preload_associations.sort_by_attribute('label_priority')
end
- def sorted_merge_requests
- merge_requests.sort_by_attribute('label_priority')
+ def sorted_merge_requests(user)
+ merge_requests_visible_to_user(user).sort_by_attribute('label_priority')
end
def upcoming?
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1e4735bac8c..b67797f5404 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -71,7 +71,7 @@ class MergeRequest < ActiveRecord::Base
serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
- after_create :ensure_merge_request_diff, unless: :importing?
+ after_create :ensure_merge_request_diff
after_update :clear_memoized_shas
after_update :reload_diff_if_branch_changed
after_save :ensure_metrics
@@ -189,6 +189,9 @@ class MergeRequest < ActiveRecord::Base
after_save :keep_around_commit
+ alias_attribute :project, :target_project
+ alias_attribute :project_id, :target_project_id
+
def self.reference_prefix
'!'
end
@@ -847,10 +850,6 @@ class MergeRequest < ActiveRecord::Base
target_project != source_project
end
- def project
- target_project
- end
-
# If the merge request closes any issues, save this information in the
# `MergeRequestsClosingIssues` model. This is a performance optimization.
# Calculating this information for a number of merge requests requires
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index e286a4e57f2..351a662ae83 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -22,6 +22,8 @@ class MergeRequestDiff < ActiveRecord::Base
has_many :merge_request_diff_commits, -> { order(:merge_request_diff_id, :relative_order) }
+ validates :base_commit_sha, :head_commit_sha, :start_commit_sha, sha: true
+
state_machine :state, initial: :empty do
event :clean do
transition any => :without_files
diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb
index 60cb2d380d5..c68a9d923c8 100644
--- a/app/models/project_services/prometheus_service.rb
+++ b/app/models/project_services/prometheus_service.rb
@@ -71,7 +71,7 @@ class PrometheusService < MonitoringService
end
def prometheus_client
- RestClient::Resource.new(api_url, max_redirects: 0) if api_url && manual_configuration? && active?
+ RestClient::Resource.new(api_url, max_redirects: 0) if should_return_client?
end
def prometheus_available?
@@ -83,6 +83,10 @@ class PrometheusService < MonitoringService
private
+ def should_return_client?
+ api_url && manual_configuration? && active? && valid?
+ end
+
def synchronize_service_state
self.active = prometheus_available? || manual_configuration?
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index 298769c0eb8..e74e5f008d7 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -53,7 +53,6 @@ class GroupPolicy < BasePolicy
rule { admin }.enable :read_group
rule { has_projects }.policy do
- enable :read_group
enable :read_list
enable :read_label
end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 533782104ac..031b72ad9c3 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -300,6 +300,8 @@ class ProjectPolicy < BasePolicy
rule { issues_disabled }.policy do
prevent(*create_read_update_admin_destroy(:issue))
+ prevent(*create_read_update_admin_destroy(:board))
+ prevent(*create_read_update_admin_destroy(:list))
end
rule { merge_requests_disabled | repository_disabled }.policy do
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index ef991eaf234..1e1f2fbd08e 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -387,4 +387,10 @@ class IssuableBaseService < BaseService
def parent
project
end
+
+ # we need to check this because milestone from milestone_id param is displayed on "new" page
+ # where private project milestone could leak without this check
+ def ensure_milestone_available(issuable)
+ issuable.milestone_id = nil unless issuable.milestone_available?
+ end
end
diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb
index 3fb2c2b3007..61615ac2058 100644
--- a/app/services/issues/build_service.rb
+++ b/app/services/issues/build_service.rb
@@ -6,7 +6,9 @@ module Issues
def execute
filter_resolve_discussion_params
- @issue = project.issues.new(issue_params)
+ @issue = project.issues.new(issue_params).tap do |issue|
+ ensure_milestone_available(issue)
+ end
end
def issue_params_with_info_from_discussions
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 48419da98ad..109c964e577 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -19,6 +19,7 @@ module MergeRequests
merge_request.target_project = find_target_project
merge_request.target_branch = find_target_branch
merge_request.can_be_created = projects_and_branches_valid?
+ ensure_milestone_available(merge_request)
# compare branches only if branches are valid, otherwise
# compare_branches may raise an error
diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb
index 1392775f805..e3d5bea0852 100644
--- a/app/services/projects/group_links/create_service.rb
+++ b/app/services/projects/group_links/create_service.rb
@@ -4,13 +4,19 @@ module Projects
module GroupLinks
class CreateService < BaseService
def execute(group)
- return false unless group
+ return error('Not Found', 404) unless group && can?(current_user, :read_namespace, group)
- project.project_group_links.create(
+ link = project.project_group_links.new(
group: group,
group_access: params[:link_group_access],
expires_at: params[:expires_at]
)
+
+ if link.save
+ success(link: link)
+ else
+ error(link.errors.full_messages.to_sentence, 409)
+ end
end
end
end
diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb
index a7f8615e9ba..236b7ed2b3d 100644
--- a/app/uploaders/file_mover.rb
+++ b/app/uploaders/file_mover.rb
@@ -11,6 +11,8 @@ class FileMover
end
def execute
+ return unless valid?
+
move
if update_markdown
@@ -21,6 +23,12 @@ class FileMover
private
+ def valid?
+ Pathname.new(temp_file_path).realpath.to_path.start_with?(
+ (Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path
+ )
+ end
+
def move
FileUtils.mkdir_p(File.dirname(file_path))
FileUtils.move(temp_file_path, file_path)
diff --git a/app/validators/sha_validator.rb b/app/validators/sha_validator.rb
new file mode 100644
index 00000000000..085fca4d65d
--- /dev/null
+++ b/app/validators/sha_validator.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class ShaValidator < ActiveModel::EachValidator
+ def validate_each(record, attribute, value)
+ return if value.blank? || value.match(/\A\h{40}\z/)
+
+ record.errors.add(attribute, 'is not a valid SHA')
+ end
+end
diff --git a/app/views/notify/issue_moved_email.html.haml b/app/views/notify/issue_moved_email.html.haml
index 472c31e9a5e..b766cb1a523 100644
--- a/app/views/notify/issue_moved_email.html.haml
+++ b/app/views/notify/issue_moved_email.html.haml
@@ -1,6 +1,9 @@
%p
Issue was moved to another project.
-%p
- New issue:
- = link_to project_issue_url(@new_project, @new_issue) do
- = @new_issue.title
+- if @can_access_project
+ %p
+ New issue:
+ = link_to project_issue_url(@new_project, @new_issue) do
+ = @new_issue.title
+- else
+ You don't have access to the project.
diff --git a/app/views/notify/issue_moved_email.text.erb b/app/views/notify/issue_moved_email.text.erb
index 66ede43635b..985e689aa9d 100644
--- a/app/views/notify/issue_moved_email.text.erb
+++ b/app/views/notify/issue_moved_email.text.erb
@@ -1,4 +1,8 @@
Issue was moved to another project.
+<% if @can_access_project %>
New issue location:
<%= project_issue_url(@new_project, @new_issue) %>
+<% else %>
+You don't have access to the project.
+<% end %>
diff --git a/app/views/profiles/active_sessions/_active_session.html.haml b/app/views/profiles/active_sessions/_active_session.html.haml
index 23ef31a0c85..2bf514d72a5 100644
--- a/app/views/profiles/active_sessions/_active_session.html.haml
+++ b/app/views/profiles/active_sessions/_active_session.html.haml
@@ -23,9 +23,3 @@
%strong Signed in
on
= l(active_session.created_at, format: :short)
-
- - unless is_current_session
- .float-right
- = link_to profile_active_session_path(active_session.session_id), data: { confirm: 'Are you sure? The device will be signed out of GitLab.' }, method: :delete, class: "btn btn-danger prepend-left-10" do
- %span.sr-only Revoke
- Revoke
diff --git a/app/views/projects/blob/viewers/_dependency_manager.html.haml b/app/views/projects/blob/viewers/_dependency_manager.html.haml
index 87aa7c1dbf8..5970d41fdab 100644
--- a/app/views/projects/blob/viewers/_dependency_manager.html.haml
+++ b/app/views/projects/blob/viewers/_dependency_manager.html.haml
@@ -3,9 +3,4 @@
This project manages its dependencies using
%strong= viewer.manager_name
- - if viewer.package_name
- and defines a #{viewer.package_type} named
- %strong<
- = link_to_if viewer.package_url.present?, viewer.package_name, viewer.package_url, target: '_blank', rel: 'noopener noreferrer'
-
= link_to 'Learn more', viewer.manager_url, target: '_blank', rel: 'noopener noreferrer'
diff --git a/changelogs/unreleased/2802-security-add-public-internal-groups-as-members-to-your-project-idor.yml b/changelogs/unreleased/2802-security-add-public-internal-groups-as-members-to-your-project-idor.yml
new file mode 100644
index 00000000000..27ad151cd06
--- /dev/null
+++ b/changelogs/unreleased/2802-security-add-public-internal-groups-as-members-to-your-project-idor.yml
@@ -0,0 +1,6 @@
+---
+title: Remove the possibility to share a project with a group that a user is not a member
+ of
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/51971-milestones-visibility.yml b/changelogs/unreleased/51971-milestones-visibility.yml
new file mode 100644
index 00000000000..818f0071e6c
--- /dev/null
+++ b/changelogs/unreleased/51971-milestones-visibility.yml
@@ -0,0 +1,5 @@
+---
+title: Check if desired milestone for an issue is available
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/57534_filter_impersonated_sessions.yml b/changelogs/unreleased/57534_filter_impersonated_sessions.yml
new file mode 100644
index 00000000000..80aea0ab1bc
--- /dev/null
+++ b/changelogs/unreleased/57534_filter_impersonated_sessions.yml
@@ -0,0 +1,6 @@
+---
+title: Do not display impersonated sessions under active sessions and remove ability
+ to revoke session
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-2797-milestone-mrs.yml b/changelogs/unreleased/security-2797-milestone-mrs.yml
new file mode 100644
index 00000000000..5bb104ec403
--- /dev/null
+++ b/changelogs/unreleased/security-2797-milestone-mrs.yml
@@ -0,0 +1,5 @@
+---
+title: Show only merge requests visible to user on milestone detail page
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-2798-fix-boards-policy.yml b/changelogs/unreleased/security-2798-fix-boards-policy.yml
new file mode 100644
index 00000000000..10e8ac3a787
--- /dev/null
+++ b/changelogs/unreleased/security-2798-fix-boards-policy.yml
@@ -0,0 +1,5 @@
+---
+title: Disable issue boards API when issues are disabled
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-2799-emails.yml b/changelogs/unreleased/security-2799-emails.yml
new file mode 100644
index 00000000000..dbf1207810e
--- /dev/null
+++ b/changelogs/unreleased/security-2799-emails.yml
@@ -0,0 +1,5 @@
+---
+title: Don't show new issue link after move when a user does not have permissions
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-50334.yml b/changelogs/unreleased/security-50334.yml
new file mode 100644
index 00000000000..828ef82b517
--- /dev/null
+++ b/changelogs/unreleased/security-50334.yml
@@ -0,0 +1,5 @@
+---
+title: Fix git clone revealing private repo's presence
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-55468-check-validity-before-querying.yml b/changelogs/unreleased/security-55468-check-validity-before-querying.yml
new file mode 100644
index 00000000000..8bb11a97f52
--- /dev/null
+++ b/changelogs/unreleased/security-55468-check-validity-before-querying.yml
@@ -0,0 +1,5 @@
+---
+title: Fix blind SSRF in Prometheus integration by checking URL before querying
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-56348.yml b/changelogs/unreleased/security-56348.yml
new file mode 100644
index 00000000000..a289e4e9077
--- /dev/null
+++ b/changelogs/unreleased/security-56348.yml
@@ -0,0 +1,5 @@
+---
+title: Check snippet attached file to be moved is within designated directory
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-commit-private-related-mr.yml b/changelogs/unreleased/security-commit-private-related-mr.yml
new file mode 100644
index 00000000000..c4de200b0d8
--- /dev/null
+++ b/changelogs/unreleased/security-commit-private-related-mr.yml
@@ -0,0 +1,5 @@
+---
+title: Don't allow non-members to see private related MRs.
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-fj-diff-import-file-read-fix.yml b/changelogs/unreleased/security-fj-diff-import-file-read-fix.yml
new file mode 100644
index 00000000000..e98d4e89712
--- /dev/null
+++ b/changelogs/unreleased/security-fj-diff-import-file-read-fix.yml
@@ -0,0 +1,5 @@
+---
+title: Fix arbitrary file read via diffs during import
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-id-restricted-access-to-private-repo.yml b/changelogs/unreleased/security-id-restricted-access-to-private-repo.yml
new file mode 100644
index 00000000000..7d7478d297b
--- /dev/null
+++ b/changelogs/unreleased/security-id-restricted-access-to-private-repo.yml
@@ -0,0 +1,5 @@
+---
+title: Forbid creating discussions for users with restricted access
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-issue_54789_2.yml b/changelogs/unreleased/security-issue_54789_2.yml
new file mode 100644
index 00000000000..8ecb72a2ae3
--- /dev/null
+++ b/changelogs/unreleased/security-issue_54789_2.yml
@@ -0,0 +1,5 @@
+---
+title: Do not disclose milestone titles for unauthorized users
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-kubernetes-google-login-csrf.yml b/changelogs/unreleased/security-kubernetes-google-login-csrf.yml
new file mode 100644
index 00000000000..2f87100a8dd
--- /dev/null
+++ b/changelogs/unreleased/security-kubernetes-google-login-csrf.yml
@@ -0,0 +1,5 @@
+---
+title: Validate session key when authorizing with GCP to create a cluster
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-kubernetes-local-ssrf.yml b/changelogs/unreleased/security-kubernetes-local-ssrf.yml
new file mode 100644
index 00000000000..7a2ad092339
--- /dev/null
+++ b/changelogs/unreleased/security-kubernetes-local-ssrf.yml
@@ -0,0 +1,5 @@
+---
+title: Block local URLs for Kubernetes integration
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-mermaid.yml b/changelogs/unreleased/security-mermaid.yml
new file mode 100644
index 00000000000..ec42b5a1615
--- /dev/null
+++ b/changelogs/unreleased/security-mermaid.yml
@@ -0,0 +1,5 @@
+---
+title: Limit mermaid rendering to 5K characters
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-osw-stop-linking-to-packages.yml b/changelogs/unreleased/security-osw-stop-linking-to-packages.yml
new file mode 100644
index 00000000000..078f06140fe
--- /dev/null
+++ b/changelogs/unreleased/security-osw-stop-linking-to-packages.yml
@@ -0,0 +1,5 @@
+---
+title: Stop linking to unrecognized package sources
+merge_request: 55518
+author:
+type: security
diff --git a/changelogs/unreleased/security-protect-private-repo-information.yml b/changelogs/unreleased/security-protect-private-repo-information.yml
new file mode 100644
index 00000000000..8b1a528206d
--- /dev/null
+++ b/changelogs/unreleased/security-protect-private-repo-information.yml
@@ -0,0 +1,5 @@
+---
+title: Fix leaking private repository information in API
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-shared-project-private-group.yml b/changelogs/unreleased/security-shared-project-private-group.yml
new file mode 100644
index 00000000000..3b21daa5491
--- /dev/null
+++ b/changelogs/unreleased/security-shared-project-private-group.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed ability to see private groups by users not belonging to given group
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-tags-oracle.yml b/changelogs/unreleased/security-tags-oracle.yml
new file mode 100644
index 00000000000..eb8ad6f646c
--- /dev/null
+++ b/changelogs/unreleased/security-tags-oracle.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent releases links API to leak tag existance
+merge_request:
+author:
+type: security
diff --git a/config/routes/git_http.rb b/config/routes/git_http.rb
index ec5c68f81df..a959d40881b 100644
--- a/config/routes/git_http.rb
+++ b/config/routes/git_http.rb
@@ -40,7 +40,7 @@ scope(path: '*namespace_id/:project_id',
# /info/refs?service=git-receive-pack, but nothing else.
#
git_http_handshake = lambda do |request|
- ::Constraints::ProjectUrlConstrainer.new.matches?(request) &&
+ ::Constraints::ProjectUrlConstrainer.new.matches?(request, existence_check: false) &&
(request.query_string.blank? ||
request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/))
end
diff --git a/doc/user/profile/active_sessions.md b/doc/user/profile/active_sessions.md
index 5119c0e30d0..28e3f4904a9 100644
--- a/doc/user/profile/active_sessions.md
+++ b/doc/user/profile/active_sessions.md
@@ -4,7 +4,7 @@
> in GitLab 10.8.
GitLab lists all devices that have logged into your account. This allows you to
-review the sessions and revoke any of it that you don't recognize.
+review the sessions.
## Listing all active sessions
@@ -12,9 +12,3 @@ review the sessions and revoke any of it that you don't recognize.
1. Navigate to the **Active Sessions** tab.
![Active sessions list](img/active_sessions_list.png)
-
-## Revoking a session
-
-1. Navigate to your [profile's](#profile-settings) **Settings > Active Sessions**.
-1. Click on **Revoke** besides a session. The current session cannot be
- revoked, as this would sign you out of GitLab.
diff --git a/doc/user/profile/img/active_sessions_list.png b/doc/user/profile/img/active_sessions_list.png
index 5d94dca69cc..1e242ac4710 100644
--- a/doc/user/profile/img/active_sessions_list.png
+++ b/doc/user/profile/img/active_sessions_list.png
Binary files differ
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index 8defc59224d..d0a9debda5b 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -318,10 +318,18 @@ module API
use :pagination
end
get ':id/repository/commits/:sha/merge_requests', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do
+ authorize! :read_merge_request, user_project
+
commit = user_project.commit(params[:sha])
not_found! 'Commit' unless commit
- present paginate(commit.merge_requests), with: Entities::MergeRequestBasic
+ commit_merge_requests = MergeRequestsFinder.new(
+ current_user,
+ project_id: user_project.id,
+ commit_sha: commit.sha
+ ).execute
+
+ present paginate(commit_merge_requests), with: Entities::MergeRequestBasic
end
desc "Get a commit's GPG signature" do
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 18f15632f2b..5176e9713c1 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -156,7 +156,7 @@ module API
class BasicProjectDetails < ProjectIdentity
include ::API::ProjectsRelationBuilder
- expose :default_branch
+ expose :default_branch, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) }
# Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770
expose :tag_list do |project|
# project.tags.order(:name).pluck(:name) is the most suitable option
@@ -261,7 +261,7 @@ module API
expose :open_issues_count, if: lambda { |project, options| project.feature_available?(:issues, options[:current_user]) }
expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] }
expose :public_builds, as: :public_jobs
- expose :ci_config_path
+ expose :ci_config_path, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) }
expose :shared_with_groups do |project, options|
SharedGroup.represent(project.project_group_links, options)
end
@@ -270,8 +270,9 @@ module API
expose :only_allow_merge_if_all_discussions_are_resolved
expose :printing_merge_request_link_enabled
expose :merge_method
-
- expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics
+ expose :statistics, using: 'API::Entities::ProjectStatistics', if: -> (project, options) {
+ options[:statistics] && Ability.allowed?(options[:current_user], :download_code, project)
+ }
# rubocop: disable CodeReuse/ActiveRecord
def self.preload_relation(projects_relation, options = {})
diff --git a/lib/api/environments.rb b/lib/api/environments.rb
index 0278c6c54a5..5b0f3b914cb 100644
--- a/lib/api/environments.rb
+++ b/lib/api/environments.rb
@@ -22,7 +22,7 @@ module API
get ':id/environments' do
authorize! :read_environment, user_project
- present paginate(user_project.environments), with: Entities::Environment
+ present paginate(user_project.environments), with: Entities::Environment, current_user: current_user
end
desc 'Creates a new environment' do
@@ -40,7 +40,7 @@ module API
environment = user_project.environments.create(declared_params)
if environment.persisted?
- present environment, with: Entities::Environment
+ present environment, with: Entities::Environment, current_user: current_user
else
render_validation_error!(environment)
end
@@ -63,7 +63,7 @@ module API
update_params = declared_params(include_missing: false).extract!(:name, :external_url)
if environment.update(update_params)
- present environment, with: Entities::Environment
+ present environment, with: Entities::Environment, current_user: current_user
else
render_validation_error!(environment)
end
@@ -99,7 +99,7 @@ module API
environment.stop_with_action!(current_user)
status 200
- present environment, with: Entities::Environment
+ present environment, with: Entities::Environment, current_user: current_user
end
end
end
diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb
index 216b2c45741..795dca5cf03 100644
--- a/lib/api/helpers/notes_helpers.rb
+++ b/lib/api/helpers/notes_helpers.rb
@@ -70,14 +70,7 @@ module API
def find_noteable(parent, noteables_str, noteable_id)
noteable = public_send("find_#{parent}_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
- readable =
- if noteable.is_a?(Commit)
- # for commits there is not :read_commit policy, check if user
- # has :read_note permission on the commit's project
- can?(current_user, :read_note, user_project)
- else
- can?(current_user, noteable_read_ability_name(noteable), noteable)
- end
+ readable = can?(current_user, noteable_read_ability_name(noteable), noteable)
return not_found!(noteables_str) unless readable
@@ -89,12 +82,11 @@ module API
end
def create_note(noteable, opts)
- policy_object = noteable.is_a?(Commit) ? user_project : noteable
- authorize!(:create_note, policy_object)
+ authorize!(:create_note, noteable)
parent = noteable_parent(noteable)
- opts.delete(:created_at) unless current_user.can?(:set_note_created_at, policy_object)
+ opts.delete(:created_at) unless current_user.can?(:set_note_created_at, noteable)
opts[:updated_at] = opts[:created_at] if opts[:created_at]
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index b23fe6cd4e7..91501ba4d36 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -184,7 +184,8 @@ module API
if project.saved?
present project, with: Entities::Project,
- user_can_admin_project: can?(current_user, :admin_project, project)
+ user_can_admin_project: can?(current_user, :admin_project, project),
+ current_user: current_user
else
if project.errors[:limit_reached].present?
error!(project.errors[:limit_reached], 403)
@@ -217,7 +218,8 @@ module API
if project.saved?
present project, with: Entities::Project,
- user_can_admin_project: can?(current_user, :admin_project, project)
+ user_can_admin_project: can?(current_user, :admin_project, project),
+ current_user: current_user
else
render_validation_error!(project)
end
@@ -281,7 +283,8 @@ module API
conflict!(forked_project.errors.messages)
else
present forked_project, with: Entities::Project,
- user_can_admin_project: can?(current_user, :admin_project, forked_project)
+ user_can_admin_project: can?(current_user, :admin_project, forked_project),
+ current_user: current_user
end
end
@@ -330,7 +333,8 @@ module API
if result[:status] == :success
present user_project, with: Entities::Project,
- user_can_admin_project: can?(current_user, :admin_project, user_project)
+ user_can_admin_project: can?(current_user, :admin_project, user_project),
+ current_user: current_user
else
render_validation_error!(user_project)
end
@@ -344,7 +348,7 @@ module API
::Projects::UpdateService.new(user_project, current_user, archived: true).execute
- present user_project, with: Entities::Project
+ present user_project, with: Entities::Project, current_user: current_user
end
desc 'Unarchive a project' do
@@ -355,7 +359,7 @@ module API
::Projects::UpdateService.new(@project, current_user, archived: false).execute
- present user_project, with: Entities::Project
+ present user_project, with: Entities::Project, current_user: current_user
end
desc 'Star a project' do
@@ -368,7 +372,7 @@ module API
current_user.toggle_star(user_project)
user_project.reload
- present user_project, with: Entities::Project
+ present user_project, with: Entities::Project, current_user: current_user
end
end
@@ -380,7 +384,7 @@ module API
current_user.toggle_star(user_project)
user_project.reload
- present user_project, with: Entities::Project
+ present user_project, with: Entities::Project, current_user: current_user
else
not_modified!
end
@@ -420,7 +424,7 @@ module API
result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project)
if result
- present user_project.reload, with: Entities::Project
+ present user_project.reload, with: Entities::Project, current_user: current_user
else
render_api_error!("Project already forked", 409) if user_project.forked?
end
@@ -442,27 +446,24 @@ module API
end
params do
requires :group_id, type: Integer, desc: 'The ID of a group'
- requires :group_access, type: Integer, values: Gitlab::Access.values, desc: 'The group access level'
+ requires :group_access, type: Integer, values: Gitlab::Access.values, as: :link_group_access, desc: 'The group access level'
optional :expires_at, type: Date, desc: 'Share expiration date'
end
post ":id/share" do
authorize! :admin_project, user_project
group = Group.find_by_id(params[:group_id])
- unless group && can?(current_user, :read_group, group)
- not_found!('Group')
- end
-
unless user_project.allowed_to_share_with_group?
break render_api_error!("The project sharing with group is disabled", 400)
end
- link = user_project.project_group_links.new(declared_params(include_missing: false))
+ result = ::Projects::GroupLinks::CreateService.new(user_project, current_user, declared_params(include_missing: false))
+ .execute(group)
- if link.save
- present link, with: Entities::ProjectGroupLink
+ if result[:status] == :success
+ present result[:link], with: Entities::ProjectGroupLink
else
- render_api_error!(link.errors.full_messages.first, 409)
+ render_api_error!(result[:message], result[:http_status])
end
end
@@ -526,7 +527,7 @@ module API
result = ::Projects::TransferService.new(user_project, current_user).execute(namespace)
if result
- present user_project, with: Entities::Project
+ present user_project, with: Entities::Project, current_user: current_user
else
render_api_error!("Failed to transfer project #{user_project.errors.messages}", 400)
end
diff --git a/lib/api/release/links.rb b/lib/api/release/links.rb
index e3072684ef7..5d1b40e3bff 100644
--- a/lib/api/release/links.rb
+++ b/lib/api/release/links.rb
@@ -8,6 +8,8 @@ module API
RELEASE_ENDPOINT_REQUIREMETS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS
.merge(tag_name: API::NO_SLASH_URL_PART_REGEX)
+ before { authorize! :read_release, user_project }
+
params do
requires :id, type: String, desc: 'The ID of a project'
end
diff --git a/lib/constraints/project_url_constrainer.rb b/lib/constraints/project_url_constrainer.rb
index eadfbf7bc01..d41490d2ebd 100644
--- a/lib/constraints/project_url_constrainer.rb
+++ b/lib/constraints/project_url_constrainer.rb
@@ -2,12 +2,13 @@
module Constraints
class ProjectUrlConstrainer
- def matches?(request)
+ def matches?(request, existence_check: true)
namespace_path = request.params[:namespace_id]
project_path = request.params[:project_id] || request.params[:id]
full_path = [namespace_path, project_path].join('/')
return false unless ProjectPathValidator.valid_path?(full_path)
+ return true unless existence_check
# We intentionally allow SELECT(*) here so result of this query can be used
# as cache for further Project.find_by_full_path calls within request
diff --git a/lib/gitlab/auth/omniauth_identity_linker_base.rb b/lib/gitlab/auth/omniauth_identity_linker_base.rb
index 253445570f2..c620fc5d6bd 100644
--- a/lib/gitlab/auth/omniauth_identity_linker_base.rb
+++ b/lib/gitlab/auth/omniauth_identity_linker_base.rb
@@ -12,7 +12,7 @@ module Gitlab
end
def link
- save if identity.new_record?
+ save if unlinked?
end
def changed?
@@ -35,6 +35,10 @@ module Gitlab
@changed = identity.save
end
+ def unlinked?
+ identity.new_record?
+ end
+
# rubocop: disable CodeReuse/ActiveRecord
def identity
@identity ||= current_user.identities
diff --git a/lib/gitlab/dependency_linker/base_linker.rb b/lib/gitlab/dependency_linker/base_linker.rb
index ac2efe598b4..ffad00fa7d7 100644
--- a/lib/gitlab/dependency_linker/base_linker.rb
+++ b/lib/gitlab/dependency_linker/base_linker.rb
@@ -4,6 +4,7 @@ module Gitlab
module DependencyLinker
class BaseLinker
URL_REGEX = %r{https?://[^'" ]+}.freeze
+ GIT_INVALID_URL_REGEX = /^git\+#{URL_REGEX}/.freeze
REPO_REGEX = %r{[^/'" ]+/[^/'" ]+}.freeze
class_attribute :file_type
@@ -29,8 +30,25 @@ module Gitlab
highlighted_lines.join.html_safe
end
+ def external_url(name, external_ref)
+ return if external_ref =~ GIT_INVALID_URL_REGEX
+
+ case external_ref
+ when /\A#{URL_REGEX}\z/
+ external_ref
+ when /\A#{REPO_REGEX}\z/
+ github_url(external_ref)
+ else
+ package_url(name)
+ end
+ end
+
private
+ def package_url(_name)
+ raise NotImplementedError
+ end
+
def link_dependencies
raise NotImplementedError
end
diff --git a/lib/gitlab/dependency_linker/composer_json_linker.rb b/lib/gitlab/dependency_linker/composer_json_linker.rb
index 22d2bead891..4b8862b31ee 100644
--- a/lib/gitlab/dependency_linker/composer_json_linker.rb
+++ b/lib/gitlab/dependency_linker/composer_json_linker.rb
@@ -8,8 +8,8 @@ module Gitlab
private
def link_packages
- link_packages_at_key("require", &method(:package_url))
- link_packages_at_key("require-dev", &method(:package_url))
+ link_packages_at_key("require")
+ link_packages_at_key("require-dev")
end
def package_url(name)
diff --git a/lib/gitlab/dependency_linker/gemfile_linker.rb b/lib/gitlab/dependency_linker/gemfile_linker.rb
index 8ab219c4962..c6e02248b0a 100644
--- a/lib/gitlab/dependency_linker/gemfile_linker.rb
+++ b/lib/gitlab/dependency_linker/gemfile_linker.rb
@@ -3,8 +3,14 @@
module Gitlab
module DependencyLinker
class GemfileLinker < MethodLinker
+ class_attribute :package_keyword
+
+ self.package_keyword = :gem
self.file_type = :gemfile
+ GITHUB_REGEX = /(github:|:github\s*=>)\s*['"](?<name>[^'"]+)['"]/.freeze
+ GIT_REGEX = /(git:|:git\s*=>)\s*['"](?<name>#{URL_REGEX})['"]/.freeze
+
private
def link_dependencies
@@ -14,21 +20,35 @@ module Gitlab
def link_urls
# Link `github: "user/repo"` to https://github.com/user/repo
- link_regex(/(github:|:github\s*=>)\s*['"](?<name>[^'"]+)['"]/, &method(:github_url))
+ link_regex(GITHUB_REGEX, &method(:github_url))
# Link `git: "https://gitlab.example.com/user/repo"` to https://gitlab.example.com/user/repo
- link_regex(/(git:|:git\s*=>)\s*['"](?<name>#{URL_REGEX})['"]/, &:itself)
+ link_regex(GIT_REGEX, &:itself)
# Link `source "https://rubygems.org"` to https://rubygems.org
link_method_call('source', URL_REGEX, &:itself)
end
def link_packages
- # Link `gem "package_name"` to https://rubygems.org/gems/package_name
- link_method_call('gem') do |name|
- "https://rubygems.org/gems/#{name}"
+ packages = parse_packages
+
+ return if packages.blank?
+
+ packages.each do |package|
+ link_method_call('gem', package.name) do
+ external_url(package.name, package.external_ref)
+ end
end
end
+
+ def package_url(name)
+ "https://rubygems.org/gems/#{name}"
+ end
+
+ def parse_packages
+ parser = Gitlab::DependencyLinker::Parser::Gemfile.new(plain_text)
+ parser.parse(keyword: self.class.package_keyword)
+ end
end
end
end
diff --git a/lib/gitlab/dependency_linker/gemspec_linker.rb b/lib/gitlab/dependency_linker/gemspec_linker.rb
index b924ea86d89..94c2b375cf9 100644
--- a/lib/gitlab/dependency_linker/gemspec_linker.rb
+++ b/lib/gitlab/dependency_linker/gemspec_linker.rb
@@ -11,7 +11,7 @@ module Gitlab
link_method_call('homepage', URL_REGEX, &:itself)
link_method_call('license', &method(:license_url))
- link_method_call(%w[name add_dependency add_runtime_dependency add_development_dependency]) do |name|
+ link_method_call(%w[add_dependency add_runtime_dependency add_development_dependency]) do |name|
"https://rubygems.org/gems/#{name}"
end
end
diff --git a/lib/gitlab/dependency_linker/method_linker.rb b/lib/gitlab/dependency_linker/method_linker.rb
index d4d85bb3390..33899a931c6 100644
--- a/lib/gitlab/dependency_linker/method_linker.rb
+++ b/lib/gitlab/dependency_linker/method_linker.rb
@@ -23,18 +23,22 @@ module Gitlab
# link_method_call('name')
# # Will link `package` in `self.name = "package"`
def link_method_call(method_name, value = nil, &url_proc)
+ regex = method_call_regex(method_name, value)
+
+ link_regex(regex, &url_proc)
+ end
+
+ def method_call_regex(method_name, value = nil)
method_name = regexp_for_value(method_name)
value = regexp_for_value(value)
- regex = %r{
+ %r{
#{method_name} # Method name
\s* # Whitespace
[(=]? # Opening brace or equals sign
\s* # Whitespace
['"](?<name>#{value})['"] # Package name in quotes
}x
-
- link_regex(regex, &url_proc)
end
end
end
diff --git a/lib/gitlab/dependency_linker/package.rb b/lib/gitlab/dependency_linker/package.rb
new file mode 100644
index 00000000000..8a509bbd562
--- /dev/null
+++ b/lib/gitlab/dependency_linker/package.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module DependencyLinker
+ class Package
+ attr_reader :name, :git_ref, :github_ref
+
+ def initialize(name, git_ref, github_ref)
+ @name = name
+ @git_ref = git_ref
+ @github_ref = github_ref
+ end
+
+ def external_ref
+ @git_ref || @github_ref
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/dependency_linker/package_json_linker.rb b/lib/gitlab/dependency_linker/package_json_linker.rb
index 578e25f806a..6857f2a4fa2 100644
--- a/lib/gitlab/dependency_linker/package_json_linker.rb
+++ b/lib/gitlab/dependency_linker/package_json_linker.rb
@@ -8,7 +8,6 @@ module Gitlab
private
def link_dependencies
- link_json('name', json["name"], &method(:package_url))
link_json('license', &method(:license_url))
link_json(%w[homepage url], URL_REGEX, &:itself)
@@ -16,25 +15,19 @@ module Gitlab
end
def link_packages
- link_packages_at_key("dependencies", &method(:package_url))
- link_packages_at_key("devDependencies", &method(:package_url))
+ link_packages_at_key("dependencies")
+ link_packages_at_key("devDependencies")
end
- def link_packages_at_key(key, &url_proc)
+ def link_packages_at_key(key)
dependencies = json[key]
return unless dependencies
dependencies.each do |name, version|
- link_json(name, version, link: :key, &url_proc)
-
- link_json(name) do |value|
- case value
- when /\A#{URL_REGEX}\z/
- value
- when /\A#{REPO_REGEX}\z/
- github_url(value)
- end
- end
+ external_url = external_url(name, version)
+
+ link_json(name, version, link: :key) { external_url }
+ link_json(name) { external_url }
end
end
diff --git a/lib/gitlab/dependency_linker/parser/gemfile.rb b/lib/gitlab/dependency_linker/parser/gemfile.rb
new file mode 100644
index 00000000000..7f755375cea
--- /dev/null
+++ b/lib/gitlab/dependency_linker/parser/gemfile.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module DependencyLinker
+ module Parser
+ class Gemfile < MethodLinker
+ GIT_REGEX = Gitlab::DependencyLinker::GemfileLinker::GIT_REGEX
+ GITHUB_REGEX = Gitlab::DependencyLinker::GemfileLinker::GITHUB_REGEX
+
+ def initialize(plain_text)
+ @plain_text = plain_text
+ end
+
+ # Returns a list of Gitlab::DependencyLinker::Package
+ #
+ # keyword - The package definition keyword, e.g. `:gem` for
+ # Gemfile parsing, `:pod` for Podfile.
+ def parse(keyword:)
+ plain_lines.each_with_object([]) do |line, packages|
+ name = fetch(line, method_call_regex(keyword))
+
+ next unless name
+
+ git_ref = fetch(line, GIT_REGEX)
+ github_ref = fetch(line, GITHUB_REGEX)
+
+ packages << Gitlab::DependencyLinker::Package.new(name, git_ref, github_ref)
+ end
+ end
+
+ private
+
+ def fetch(line, regex, group: :name)
+ match = line.match(regex)
+ match[group] if match
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/dependency_linker/podfile_linker.rb b/lib/gitlab/dependency_linker/podfile_linker.rb
index def9b04cca9..a20d285da79 100644
--- a/lib/gitlab/dependency_linker/podfile_linker.rb
+++ b/lib/gitlab/dependency_linker/podfile_linker.rb
@@ -5,12 +5,21 @@ module Gitlab
class PodfileLinker < GemfileLinker
include Cocoapods
+ self.package_keyword = :pod
self.file_type = :podfile
private
def link_packages
- link_method_call('pod', &method(:package_url))
+ packages = parse_packages
+
+ return unless packages
+
+ packages.each do |package|
+ link_method_call('pod', package.name) do
+ external_url(package.name, package.external_ref)
+ end
+ end
end
end
end
diff --git a/lib/gitlab/dependency_linker/podspec_linker.rb b/lib/gitlab/dependency_linker/podspec_linker.rb
index 6b1758c5a43..14abd3999c4 100644
--- a/lib/gitlab/dependency_linker/podspec_linker.rb
+++ b/lib/gitlab/dependency_linker/podspec_linker.rb
@@ -19,7 +19,7 @@ module Gitlab
link_method_call('license', &method(:license_url))
link_regex(/license\s*=\s*\{\s*(type:|:type\s*=>)\s*#{STRING_REGEX}/, &method(:license_url))
- link_method_call(%w[name dependency], &method(:package_url))
+ link_method_call('dependency', &method(:package_url))
end
end
end
diff --git a/lib/gitlab/import_export/merge_request_parser.rb b/lib/gitlab/import_export/merge_request_parser.rb
index 040a70d6775..deb2f59f05f 100644
--- a/lib/gitlab/import_export/merge_request_parser.rb
+++ b/lib/gitlab/import_export/merge_request_parser.rb
@@ -20,6 +20,17 @@ module Gitlab
create_target_branch unless branch_exists?(@merge_request.target_branch)
end
+ # The merge_request_diff associated with the current @merge_request might
+ # be invalid. Than means, when the @merge_request object is saved, the
+ # @merge_request.merge_request_diff won't. This can leave the merge request
+ # in an invalid state, because a merge request must have an associated
+ # merge request diff.
+ # In this change, if the associated merge request diff is invalid, we set
+ # it to nil. This change, in association with the after callback
+ # :ensure_merge_request_diff in the MergeRequest class, makes that
+ # when the merge request is going to be created and it doesn't have
+ # one, a default one will be generated.
+ @merge_request.merge_request_diff = nil unless @merge_request.merge_request_diff&.valid?
@merge_request
end
diff --git a/lib/gitlab/kubernetes/kube_client.rb b/lib/gitlab/kubernetes/kube_client.rb
index 624c2c67551..de14df56555 100644
--- a/lib/gitlab/kubernetes/kube_client.rb
+++ b/lib/gitlab/kubernetes/kube_client.rb
@@ -82,6 +82,8 @@ module Gitlab
def initialize(api_prefix, **kubeclient_options)
@api_prefix = api_prefix
@kubeclient_options = kubeclient_options.merge(http_max_redirects: 0)
+
+ validate_url!
end
def create_or_update_cluster_role_binding(resource)
@@ -118,6 +120,12 @@ module Gitlab
private
+ def validate_url!
+ return if Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
+
+ Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false)
+ end
+
def cluster_role_binding_exists?(resource)
get_cluster_role_binding(resource.metadata.name)
rescue ::Kubeclient::ResourceNotFoundError
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index c4bcc1172eb..3abd570175e 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1341,6 +1341,9 @@ msgstr ""
msgid "Cannot modify managed Kubernetes cluster"
msgstr ""
+msgid "Cannot render the image. Maximum character count (%{charLimit}) has been exceeded."
+msgstr ""
+
msgid "Certificate"
msgstr ""
diff --git a/spec/controllers/dashboard/milestones_controller_spec.rb b/spec/controllers/dashboard/milestones_controller_spec.rb
index 4b164d0aa6b..ab40b4eb178 100644
--- a/spec/controllers/dashboard/milestones_controller_spec.rb
+++ b/spec/controllers/dashboard/milestones_controller_spec.rb
@@ -13,7 +13,7 @@ describe Dashboard::MilestonesController do
)
end
let(:issue) { create(:issue, project: project, milestone: project_milestone) }
- let(:group_issue) { create(:issue, milestone: group_milestone) }
+ let(:group_issue) { create(:issue, milestone: group_milestone, project: create(:project, group: group)) }
let!(:label) { create(:label, project: project, title: 'Issue Label', issues: [issue]) }
let!(:group_label) { create(:group_label, group: group, title: 'Group Issue Label', issues: [group_issue]) }
diff --git a/spec/controllers/google_api/authorizations_controller_spec.rb b/spec/controllers/google_api/authorizations_controller_spec.rb
index 1e8e82da4f3..d9ba85cf56a 100644
--- a/spec/controllers/google_api/authorizations_controller_spec.rb
+++ b/spec/controllers/google_api/authorizations_controller_spec.rb
@@ -6,7 +6,7 @@ describe GoogleApi::AuthorizationsController do
let(:token) { 'token' }
let(:expires_at) { 1.hour.since.strftime('%s') }
- subject { get :callback, params: { code: 'xxx', state: @state } }
+ subject { get :callback, params: { code: 'xxx', state: state } }
before do
sign_in(user)
@@ -15,35 +15,57 @@ describe GoogleApi::AuthorizationsController do
.to receive(:get_token).and_return([token, expires_at])
end
- it 'sets token and expires_at in session' do
- subject
+ shared_examples_for 'access denied' do
+ it 'returns a 404' do
+ subject
- expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token])
- .to eq(token)
- expect(session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at])
- .to eq(expires_at)
+ expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token]).to be_nil
+ expect(response).to have_http_status(:not_found)
+ end
end
- context 'when redirect uri key is stored in state' do
- set(:project) { create(:project) }
- let(:redirect_uri) { project_clusters_url(project).to_s }
+ context 'session key is present' do
+ let(:session_key) { 'session-key' }
+ let(:redirect_uri) { 'example.com' }
before do
- @state = GoogleApi::CloudPlatform::Client
- .new_session_key_for_redirect_uri do |key|
- session[key] = redirect_uri
+ session[GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(session_key)] = redirect_uri
+ end
+
+ context 'session key matches state param' do
+ let(:state) { session_key }
+
+ it 'sets token and expires_at in session' do
+ subject
+
+ expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token])
+ .to eq(token)
+ expect(session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at])
+ .to eq(expires_at)
+ end
+
+ it 'redirects to the URL stored in state param' do
+ expect(subject).to redirect_to(redirect_uri)
end
end
- it 'redirects to the URL stored in state param' do
- expect(subject).to redirect_to(redirect_uri)
+ context 'session key does not match state param' do
+ let(:state) { 'bad-key' }
+
+ it_behaves_like 'access denied'
end
- end
- context 'when redirection url is not stored in state' do
- it 'redirects to root_path' do
- expect(subject).to redirect_to(root_path)
+ context 'state param is blank' do
+ let(:state) { '' }
+
+ it_behaves_like 'access denied'
end
end
+
+ context 'state param is present, but session key is blank' do
+ let(:state) { 'session-key' }
+
+ it_behaves_like 'access denied'
+ end
end
end
diff --git a/spec/controllers/groups/shared_projects_controller_spec.rb b/spec/controllers/groups/shared_projects_controller_spec.rb
index dab7700cf64..b0c20fb5a90 100644
--- a/spec/controllers/groups/shared_projects_controller_spec.rb
+++ b/spec/controllers/groups/shared_projects_controller_spec.rb
@@ -6,6 +6,8 @@ describe Groups::SharedProjectsController do
end
def share_project(project)
+ group.add_developer(user)
+
Projects::GroupLinks::CreateService.new(
project,
user,
diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb
index 232a5e2793b..e0da23ca0b8 100644
--- a/spec/controllers/omniauth_callbacks_controller_spec.rb
+++ b/spec/controllers/omniauth_callbacks_controller_spec.rb
@@ -193,7 +193,7 @@ describe OmniauthCallbacksController, type: :controller do
before do
stub_omniauth_saml_config({ enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'],
providers: [saml_config] })
- mock_auth_hash('saml', 'my-uid', user.email, mock_saml_response)
+ mock_auth_hash_with_saml_xml('saml', 'my-uid', user.email, mock_saml_response)
request.env["devise.mapping"] = Devise.mappings[:user]
request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth']
post :saml, params: { SAMLResponse: mock_saml_response }
diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb
index 4bc72042710..a9a058e7e17 100644
--- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb
+++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb
@@ -35,4 +35,35 @@ describe Projects::AutocompleteSourcesController do
avatar_url: user.avatar_url)
end
end
+
+ describe 'GET milestones' do
+ let(:group) { create(:group, :public) }
+ let(:project) { create(:project, :public, namespace: group) }
+ let!(:project_milestone) { create(:milestone, project: project) }
+ let!(:group_milestone) { create(:milestone, group: group) }
+
+ before do
+ sign_in(user)
+ end
+
+ it 'lists milestones' do
+ group.add_owner(user)
+
+ get :milestones, format: :json, params: { namespace_id: group.path, project_id: project.path }
+
+ milestone_titles = json_response.map { |milestone| milestone["title"] }
+ expect(milestone_titles).to match_array([project_milestone.title, group_milestone.title])
+ end
+
+ context 'when user cannot read project issues and merge requests' do
+ it 'renders 404' do
+ project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
+ project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+
+ get :milestones, format: :json, params: { namespace_id: group.path, project_id: project.path }
+
+ expect(response).to have_gitlab_http_status(404)
+ end
+ end
+ end
end
diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb
index 675eeff8d12..ce021b2f085 100644
--- a/spec/controllers/projects/group_links_controller_spec.rb
+++ b/spec/controllers/projects/group_links_controller_spec.rb
@@ -65,8 +65,24 @@ describe Projects::GroupLinksController do
end
end
+ context 'when user does not have access to the public group' do
+ let(:group) { create(:group, :public) }
+
+ include_context 'link project to group'
+
+ it 'renders 404' do
+ expect(response.status).to eq 404
+ end
+
+ it 'does not share project with that group' do
+ expect(group.shared_projects).not_to include project
+ end
+ end
+
context 'when project group id equal link group id' do
before do
+ group2.add_developer(user)
+
post(:create, params: {
namespace_id: project.namespace,
project_id: project,
@@ -102,5 +118,26 @@ describe Projects::GroupLinksController do
expect(flash[:alert]).to eq('Please select a group.')
end
end
+
+ context 'when link is not persisted in the database' do
+ before do
+ allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute)
+ .and_return({ status: :error, http_status: 409, message: 'error' })
+
+ post(:create, params: {
+ namespace_id: project.namespace,
+ project_id: project,
+ link_group_id: group.id,
+ link_group_access: ProjectGroupLink.default_access
+ })
+ end
+
+ it 'redirects to project group links page' do
+ expect(response).to redirect_to(
+ project_project_members_path(project)
+ )
+ expect(flash[:alert]).to eq('error')
+ end
+ end
end
end
diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb
index 5c6858dc7b2..77a94f26d8c 100644
--- a/spec/controllers/snippets_controller_spec.rb
+++ b/spec/controllers/snippets_controller_spec.rb
@@ -205,6 +205,8 @@ describe SnippetsController do
end
context 'when the snippet description contains a file' do
+ include FileMoverHelpers
+
let(:picture_file) { '/-/system/temp/secret56/picture.jpg' }
let(:text_file) { '/-/system/temp/secret78/text.txt' }
let(:description) do
@@ -215,6 +217,8 @@ describe SnippetsController do
before do
allow(FileUtils).to receive(:mkdir_p)
allow(FileUtils).to receive(:move)
+ stub_file_mover(text_file)
+ stub_file_mover(picture_file)
end
subject { create_snippet({ description: description }, { files: [picture_file, text_file] }) }
diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb
index 406e80e91aa..9bc340ed4bb 100644
--- a/spec/features/issues_spec.rb
+++ b/spec/features/issues_spec.rb
@@ -233,8 +233,8 @@ describe 'Issues' do
created_at: Time.now - (index * 60))
end
end
- let(:newer_due_milestone) { create(:milestone, due_date: '2013-12-11') }
- let(:later_due_milestone) { create(:milestone, due_date: '2013-12-12') }
+ let(:newer_due_milestone) { create(:milestone, project: project, due_date: '2013-12-11') }
+ let(:later_due_milestone) { create(:milestone, project: project, due_date: '2013-12-12') }
it 'sorts by newest' do
visit project_issues_path(project, sort: sort_value_created_date)
diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb
index aa91ade46ca..5c45e363997 100644
--- a/spec/features/merge_request/user_sees_versions_spec.rb
+++ b/spec/features/merge_request/user_sees_versions_spec.rb
@@ -1,7 +1,11 @@
require 'rails_helper'
describe 'Merge request > User sees versions', :js do
- let(:merge_request) { create(:merge_request, importing: true) }
+ let(:merge_request) do
+ create(:merge_request).tap do |mr|
+ mr.merge_request_diff.destroy
+ end
+ end
let(:project) { merge_request.source_project }
let(:user) { project.creator }
let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }
diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb
index ef7ae490b0f..c691011b9ca 100644
--- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb
+++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb
@@ -13,7 +13,7 @@ describe 'Merge requests > User lists merge requests' do
source_project: project,
source_branch: 'fix',
assignee: user,
- milestone: create(:milestone, due_date: '2013-12-11'),
+ milestone: create(:milestone, project: project, due_date: '2013-12-11'),
created_at: 1.minute.ago,
updated_at: 1.minute.ago)
create(:merge_request,
@@ -21,7 +21,7 @@ describe 'Merge requests > User lists merge requests' do
source_project: project,
source_branch: 'markdown',
assignee: user,
- milestone: create(:milestone, due_date: '2013-12-12'),
+ milestone: create(:milestone, project: project, due_date: '2013-12-12'),
created_at: 2.minutes.ago,
updated_at: 2.minutes.ago)
create(:merge_request,
diff --git a/spec/features/profiles/active_sessions_spec.rb b/spec/features/profiles/active_sessions_spec.rb
index d3050760c06..2aa0177af5d 100644
--- a/spec/features/profiles/active_sessions_spec.rb
+++ b/spec/features/profiles/active_sessions_spec.rb
@@ -7,6 +7,8 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do
end
end
+ let(:admin) { create(:admin) }
+
around do |example|
Timecop.freeze(Time.zone.parse('2018-03-12 09:06')) do
example.run
@@ -16,6 +18,7 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do
it 'User sees their active sessions' do
Capybara::Session.new(:session1)
Capybara::Session.new(:session2)
+ Capybara::Session.new(:session3)
# note: headers can only be set on the non-js (aka. rack-test) driver
using_session :session1 do
@@ -37,9 +40,27 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do
gitlab_sign_in(user)
end
+ # set an admin session impersonating the user
+ using_session :session3 do
+ Capybara.page.driver.header(
+ 'User-Agent',
+ 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36'
+ )
+
+ gitlab_sign_in(admin)
+
+ visit admin_user_path(user)
+
+ click_link 'Impersonate'
+ end
+
using_session :session1 do
visit profile_active_sessions_path
+ expect(page).to(
+ have_selector('ul.list-group li.list-group-item', { text: 'Signed in on',
+ count: 2 }))
+
expect(page).to have_content(
'127.0.0.1 ' \
'This is your current session ' \
@@ -57,33 +78,8 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do
)
expect(page).to have_selector '[title="Smartphone"]', count: 1
- end
- end
-
- it 'User can revoke a session', :js, :redis_session_store do
- Capybara::Session.new(:session1)
- Capybara::Session.new(:session2)
-
- # set an additional session in another browser
- using_session :session2 do
- gitlab_sign_in(user)
- end
-
- using_session :session1 do
- gitlab_sign_in(user)
- visit profile_active_sessions_path
-
- expect(page).to have_link('Revoke', count: 1)
-
- accept_confirm { click_on 'Revoke' }
-
- expect(page).not_to have_link('Revoke')
- end
-
- using_session :session2 do
- visit profile_active_sessions_path
- expect(page).to have_content('You need to sign in or sign up before continuing.')
+ expect(page).not_to have_content('Chrome on Windows')
end
end
end
diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb
index 3edcc7ac2cd..a7aa63018fd 100644
--- a/spec/features/projects/blobs/blob_show_spec.rb
+++ b/spec/features/projects/blobs/blob_show_spec.rb
@@ -548,10 +548,7 @@ describe 'File blob', :js do
it 'displays an auxiliary viewer' do
aggregate_failures do
# shows names of dependency manager and package
- expect(page).to have_content('This project manages its dependencies using RubyGems and defines a gem named activerecord.')
-
- # shows a link to the gem
- expect(page).to have_link('activerecord', href: 'https://rubygems.org/gems/activerecord')
+ expect(page).to have_content('This project manages its dependencies using RubyGems.')
# shows a learn more link
expect(page).to have_link('Learn more', href: 'https://rubygems.org/')
diff --git a/spec/features/projects/members/invite_group_spec.rb b/spec/features/projects/members/invite_group_spec.rb
index fceead0b45e..b2d2dba55f1 100644
--- a/spec/features/projects/members/invite_group_spec.rb
+++ b/spec/features/projects/members/invite_group_spec.rb
@@ -27,6 +27,7 @@ describe 'Project > Members > Invite group', :js do
before do
project.add_maintainer(maintainer)
+ group_to_share_with.add_guest(maintainer)
sign_in(maintainer)
end
@@ -112,6 +113,7 @@ describe 'Project > Members > Invite group', :js do
before do
project.add_maintainer(maintainer)
+ group.add_guest(maintainer)
sign_in(maintainer)
visit project_settings_members_path(project)
diff --git a/spec/features/projects/settings/user_manages_group_links_spec.rb b/spec/features/projects/settings/user_manages_group_links_spec.rb
index 676659b90c3..e5a58c44e41 100644
--- a/spec/features/projects/settings/user_manages_group_links_spec.rb
+++ b/spec/features/projects/settings/user_manages_group_links_spec.rb
@@ -10,6 +10,7 @@ describe 'Projects > Settings > User manages group links' do
before do
project.add_maintainer(user)
+ group_market.add_guest(user)
sign_in(user)
share_link = project.project_group_links.new(group_access: Gitlab::Access::MAINTAINER)
diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb
index 4705cd12d23..3238e07fe15 100644
--- a/spec/features/security/group/private_access_spec.rb
+++ b/spec/features/security/group/private_access_spec.rb
@@ -27,7 +27,7 @@ describe 'Private Group access' do
it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) }
- it { is_expected.to be_allowed_for(project_guest) }
+ it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) }
@@ -42,7 +42,7 @@ describe 'Private Group access' do
it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) }
- it { is_expected.to be_allowed_for(project_guest) }
+ it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) }
@@ -58,7 +58,7 @@ describe 'Private Group access' do
it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) }
- it { is_expected.to be_allowed_for(project_guest) }
+ it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) }
@@ -73,7 +73,7 @@ describe 'Private Group access' do
it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) }
- it { is_expected.to be_allowed_for(project_guest) }
+ it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) }
@@ -93,4 +93,28 @@ describe 'Private Group access' do
it { is_expected.to be_denied_for(:visitor) }
it { is_expected.to be_denied_for(:external) }
end
+
+ describe 'GET /groups/:path for shared projects' do
+ let(:project) { create(:project, :public) }
+ before do
+ Projects::GroupLinks::CreateService.new(
+ project,
+ create(:user),
+ link_group_access: ProjectGroupLink::DEVELOPER
+ ).execute(group)
+ end
+
+ subject { group_path(group) }
+
+ it { is_expected.to be_allowed_for(:admin) }
+ it { is_expected.to be_allowed_for(:owner).of(group) }
+ it { is_expected.to be_allowed_for(:maintainer).of(group) }
+ it { is_expected.to be_allowed_for(:developer).of(group) }
+ it { is_expected.to be_allowed_for(:reporter).of(group) }
+ it { is_expected.to be_allowed_for(:guest).of(group) }
+ it { is_expected.to be_denied_for(project_guest) }
+ it { is_expected.to be_denied_for(:user) }
+ it { is_expected.to be_denied_for(:external) }
+ it { is_expected.to be_denied_for(:visitor) }
+ end
end
diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb
index 107da08a0a9..79f854cdb96 100644
--- a/spec/finders/merge_requests_finder_spec.rb
+++ b/spec/finders/merge_requests_finder_spec.rb
@@ -31,7 +31,7 @@ describe MergeRequestsFinder do
p
end
end
- let(:project4) { create_project_without_n_plus_1(group: subgroup) }
+ let(:project4) { create_project_without_n_plus_1(:repository, group: subgroup) }
let(:project5) { create_project_without_n_plus_1(group: subgroup) }
let(:project6) { create_project_without_n_plus_1(group: subgroup) }
@@ -68,6 +68,15 @@ describe MergeRequestsFinder do
expect(merge_requests.size).to eq(2)
end
+ it 'filters by commit sha' do
+ merge_requests = described_class.new(
+ user,
+ commit_sha: merge_request5.merge_request_diff.last_commit_sha
+ ).execute
+
+ expect(merge_requests).to contain_exactly(merge_request5)
+ end
+
context 'filtering by group' do
it 'includes all merge requests when user has access' do
params = { group_id: group.id }
@@ -269,6 +278,21 @@ describe MergeRequestsFinder do
expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request)
end
end
+
+ context 'when project restricts merge requests' do
+ let(:non_member) { create(:user) }
+ let(:project) { create(:project, :repository, :public, :merge_requests_private) }
+ let!(:merge_request) { create(:merge_request, source_project: project) }
+
+ it "returns nothing to to non members" do
+ merge_requests = described_class.new(
+ non_member,
+ project_id: project.id
+ ).execute
+
+ expect(merge_requests).to be_empty
+ end
+ end
end
describe '#row_count', :request_store do
diff --git a/spec/lib/constraints/project_url_constrainer_spec.rb b/spec/lib/constraints/project_url_constrainer_spec.rb
index c96e7ab8495..3496b01ebcc 100644
--- a/spec/lib/constraints/project_url_constrainer_spec.rb
+++ b/spec/lib/constraints/project_url_constrainer_spec.rb
@@ -16,6 +16,10 @@ describe Constraints::ProjectUrlConstrainer do
let(:request) { build_request('foo', 'bar') }
it { expect(subject.matches?(request)).to be_falsey }
+
+ context 'existence_check is false' do
+ it { expect(subject.matches?(request, existence_check: false)).to be_truthy }
+ end
end
context "project id ending with .git" do
diff --git a/spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb b/spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb
index 4d222564fd0..0ebd8994636 100644
--- a/spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb
+++ b/spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb
@@ -50,8 +50,8 @@ describe Gitlab::DependencyLinker::ComposerJsonLinker do
%{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>}
end
- it 'links the module name' do
- expect(subject).to include(link('laravel/laravel', 'https://packagist.org/packages/laravel/laravel'))
+ it 'does not link the module name' do
+ expect(subject).not_to include(link('laravel/laravel', 'https://packagist.org/packages/laravel/laravel'))
end
it 'links the homepage' do
diff --git a/spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb b/spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb
index a97803b119e..f00f6b47b94 100644
--- a/spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb
+++ b/spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb
@@ -41,13 +41,16 @@ describe Gitlab::DependencyLinker::GemfileLinker do
end
it 'links dependencies' do
- expect(subject).to include(link('rails', 'https://rubygems.org/gems/rails'))
expect(subject).to include(link('rails-deprecated_sanitizer', 'https://rubygems.org/gems/rails-deprecated_sanitizer'))
- expect(subject).to include(link('responders', 'https://rubygems.org/gems/responders'))
- expect(subject).to include(link('sprockets', 'https://rubygems.org/gems/sprockets'))
expect(subject).to include(link('default_value_for', 'https://rubygems.org/gems/default_value_for'))
end
+ it 'links to external dependencies' do
+ expect(subject).to include(link('rails', 'https://github.com/rails/rails'))
+ expect(subject).to include(link('responders', 'https://github.com/rails/responders'))
+ expect(subject).to include(link('sprockets', 'https://gitlab.example.com/gems/sprockets'))
+ end
+
it 'links GitHub repos' do
expect(subject).to include(link('rails/rails', 'https://github.com/rails/rails'))
expect(subject).to include(link('rails/responders', 'https://github.com/rails/responders'))
diff --git a/spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb b/spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb
index 24ad7d12f4c..6c6a5d70576 100644
--- a/spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb
+++ b/spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb
@@ -43,8 +43,8 @@ describe Gitlab::DependencyLinker::GemspecLinker do
%{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>}
end
- it 'links the gem name' do
- expect(subject).to include(link('gitlab_git', 'https://rubygems.org/gems/gitlab_git'))
+ it 'does not link the gem name' do
+ expect(subject).not_to include(link('gitlab_git', 'https://rubygems.org/gems/gitlab_git'))
end
it 'links the license' do
diff --git a/spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb b/spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb
index 1e8b72afb7b..9050127af7f 100644
--- a/spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb
+++ b/spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb
@@ -33,7 +33,8 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do
"express": "4.2.x",
"bigpipe": "bigpipe/pagelet",
"plates": "https://github.com/flatiron/plates/tarball/master",
- "karma": "^1.4.1"
+ "karma": "^1.4.1",
+ "random": "git+https://EdOverflow@github.com/example/example.git"
},
"devDependencies": {
"vows": "^0.7.0",
@@ -51,8 +52,8 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do
%{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>}
end
- it 'links the module name' do
- expect(subject).to include(link('module-name', 'https://npmjs.com/package/module-name'))
+ it 'does not link the module name' do
+ expect(subject).not_to include(link('module-name', 'https://npmjs.com/package/module-name'))
end
it 'links the homepage' do
@@ -71,14 +72,21 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do
expect(subject).to include(link('primus', 'https://npmjs.com/package/primus'))
expect(subject).to include(link('async', 'https://npmjs.com/package/async'))
expect(subject).to include(link('express', 'https://npmjs.com/package/express'))
- expect(subject).to include(link('bigpipe', 'https://npmjs.com/package/bigpipe'))
- expect(subject).to include(link('plates', 'https://npmjs.com/package/plates'))
expect(subject).to include(link('karma', 'https://npmjs.com/package/karma'))
expect(subject).to include(link('vows', 'https://npmjs.com/package/vows'))
expect(subject).to include(link('assume', 'https://npmjs.com/package/assume'))
expect(subject).to include(link('pre-commit', 'https://npmjs.com/package/pre-commit'))
end
+ it 'links dependencies to URL detected on value' do
+ expect(subject).to include(link('bigpipe', 'https://github.com/bigpipe/pagelet'))
+ expect(subject).to include(link('plates', 'https://github.com/flatiron/plates/tarball/master'))
+ end
+
+ it 'does not link to NPM when invalid git URL' do
+ expect(subject).not_to include(link('random', 'https://npmjs.com/package/random'))
+ end
+
it 'links GitHub repos' do
expect(subject).to include(link('bigpipe/pagelet', 'https://github.com/bigpipe/pagelet'))
end
diff --git a/spec/lib/gitlab/dependency_linker/parser/gemfile_spec.rb b/spec/lib/gitlab/dependency_linker/parser/gemfile_spec.rb
new file mode 100644
index 00000000000..9bfb1b13a2b
--- /dev/null
+++ b/spec/lib/gitlab/dependency_linker/parser/gemfile_spec.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Gitlab::DependencyLinker::Parser::Gemfile do
+ describe '#parse' do
+ let(:file_content) do
+ <<-CONTENT.strip_heredoc
+ source 'https://rubygems.org'
+
+ gem "rails", '4.2.6', github: "rails/rails"
+ gem 'rails-deprecated_sanitizer', '~> 1.0.3'
+ gem 'responders', '~> 2.0', :github => 'rails/responders'
+ gem 'sprockets', '~> 3.6.0', git: 'https://gitlab.example.com/gems/sprockets'
+ gem 'default_value_for', '~> 3.0.0'
+ CONTENT
+ end
+
+ subject { described_class.new(file_content).parse(keyword: 'gem') }
+
+ def fetch_package(name)
+ subject.find { |package| package.name == name }
+ end
+
+ it 'returns parsed packages' do
+ expect(subject.size).to eq(5)
+ expect(subject).to all(be_a(Gitlab::DependencyLinker::Package))
+ end
+
+ it 'packages respond to name and external_ref accordingly' do
+ expect(fetch_package('rails')).to have_attributes(name: 'rails',
+ github_ref: 'rails/rails',
+ git_ref: nil)
+
+ expect(fetch_package('sprockets')).to have_attributes(name: 'sprockets',
+ github_ref: nil,
+ git_ref: 'https://gitlab.example.com/gems/sprockets')
+
+ expect(fetch_package('default_value_for')).to have_attributes(name: 'default_value_for',
+ github_ref: nil,
+ git_ref: nil)
+ end
+ end
+end
diff --git a/spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb b/spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb
index cdfd7ad9826..8f1b523653e 100644
--- a/spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb
+++ b/spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb
@@ -43,7 +43,10 @@ describe Gitlab::DependencyLinker::PodfileLinker do
it 'links packages' do
expect(subject).to include(link('AFNetworking', 'https://cocoapods.org/pods/AFNetworking'))
- expect(subject).to include(link('Interstellar/Core', 'https://cocoapods.org/pods/Interstellar'))
+ end
+
+ it 'links external packages' do
+ expect(subject).to include(link('Interstellar/Core', 'https://github.com/ashfurrow/Interstellar.git'))
end
it 'links Git repos' do
diff --git a/spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb b/spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb
index ed60ab45955..bacec830103 100644
--- a/spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb
+++ b/spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb
@@ -42,8 +42,8 @@ describe Gitlab::DependencyLinker::PodspecLinker do
%{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>}
end
- it 'links the gem name' do
- expect(subject).to include(link('Reachability', 'https://cocoapods.org/pods/Reachability'))
+ it 'does not link the pod name' do
+ expect(subject).not_to include(link('Reachability', 'https://cocoapods.org/pods/Reachability'))
end
it 'links the license' do
diff --git a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb
index 68eaa70e6b6..4b234411a44 100644
--- a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb
+++ b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb
@@ -41,4 +41,20 @@ describe Gitlab::ImportExport::MergeRequestParser do
expect(parsed_merge_request).to eq(merge_request)
end
+
+ context 'when the merge request has diffs' do
+ let(:merge_request) do
+ build(:merge_request, source_project: forked_project, target_project: project)
+ end
+
+ context 'when the diff is invalid' do
+ let(:merge_request_diff) { build(:merge_request_diff, merge_request: merge_request, base_commit_sha: 'foobar') }
+
+ it 'sets the diff to nil' do
+ expect(merge_request_diff).to be_invalid
+ expect(merge_request_diff.merge_request).to eq merge_request
+ expect(parsed_merge_request.merge_request_diff).to be_nil
+ end
+ end
+ end
end
diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb
index 02364e92149..978e64c4407 100644
--- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb
+++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb
@@ -50,6 +50,36 @@ describe Gitlab::Kubernetes::KubeClient do
end
end
+ describe '#initialize' do
+ shared_examples 'local address' do
+ it 'blocks local addresses' do
+ expect { client }.to raise_error(Gitlab::UrlBlocker::BlockedUrlError)
+ end
+
+ context 'when local requests are allowed' do
+ before do
+ stub_application_setting(allow_local_requests_from_hooks_and_services: true)
+ end
+
+ it 'allows local addresses' do
+ expect { client }.not_to raise_error
+ end
+ end
+ end
+
+ context 'localhost address' do
+ let(:api_url) { 'http://localhost:22' }
+
+ it_behaves_like 'local address'
+ end
+
+ context 'private network address' do
+ let(:api_url) { 'http://192.168.1.2:3003' }
+
+ it_behaves_like 'local address'
+ end
+ end
+
describe '#core_client' do
subject { client.core_client }
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 15b04c9d066..3c8897ed37c 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -208,25 +208,53 @@ describe Notify do
let(:new_issue) { create(:issue) }
subject { described_class.issue_moved_email(recipient, issue, new_issue, current_user) }
- it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
- let(:model) { issue }
- end
- it_behaves_like 'it should show Gmail Actions View Issue link'
- it_behaves_like 'an unsubscribeable thread'
- it_behaves_like 'appearance header and footer enabled'
- it_behaves_like 'appearance header and footer not enabled'
+ context 'when a user has permissions to access the new issue' do
+ before do
+ new_issue.project.add_developer(recipient)
+ end
+
+ it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+ let(:model) { issue }
+ end
+ it_behaves_like 'it should show Gmail Actions View Issue link'
+ it_behaves_like 'an unsubscribeable thread'
+
+ it 'contains description about action taken' do
+ is_expected.to have_body_text 'Issue was moved to another project'
+ end
- it 'contains description about action taken' do
- is_expected.to have_body_text 'Issue was moved to another project'
+ it 'has the correct subject and body' do
+ new_issue_url = project_issue_path(new_issue.project, new_issue)
+
+ aggregate_failures do
+ is_expected.to have_referable_subject(issue, reply: true)
+ is_expected.to have_body_text(new_issue_url)
+ is_expected.to have_body_text(project_issue_path(project, issue))
+ end
+ end
+
+ it 'contains the issue title' do
+ is_expected.to have_body_text new_issue.title
+ end
end
- it 'has the correct subject and body' do
- new_issue_url = project_issue_path(new_issue.project, new_issue)
+ context 'when a user does not permissions to access the new issue' do
+ it 'has the correct subject and body' do
+ new_issue_url = project_issue_path(new_issue.project, new_issue)
- aggregate_failures do
- is_expected.to have_referable_subject(issue, reply: true)
- is_expected.to have_body_text(new_issue_url)
- is_expected.to have_body_text(project_issue_path(project, issue))
+ aggregate_failures do
+ is_expected.to have_referable_subject(issue, reply: true)
+ is_expected.not_to have_body_text(new_issue_url)
+ is_expected.to have_body_text(project_issue_path(project, issue))
+ end
+ end
+
+ it 'does not contain the issue title' do
+ is_expected.not_to have_body_text new_issue.title
+ end
+
+ it 'contains information about missing permissions' do
+ is_expected.to have_body_text "You don't have access to the project."
end
end
end
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb
index 129b2f92683..e128fe8a4b7 100644
--- a/spec/models/active_session_spec.rb
+++ b/spec/models/active_session_spec.rb
@@ -7,7 +7,10 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
end
end
- let(:session) { double(:session, id: '6919a6f1bb119dd7396fadc38fd18d0d') }
+ let(:session) do
+ double(:session, { id: '6919a6f1bb119dd7396fadc38fd18d0d',
+ '[]': {} })
+ end
let(:request) do
double(:request, {
diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb
index 4068d98d8f7..3b32ca8df05 100644
--- a/spec/models/clusters/platforms/kubernetes_spec.rb
+++ b/spec/models/clusters/platforms/kubernetes_spec.rb
@@ -98,6 +98,22 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
it { expect(kubernetes.save).to be_truthy }
end
+
+ context 'when api_url is localhost' do
+ let(:api_url) { 'http://localhost:22' }
+
+ it { expect(kubernetes.save).to be_falsey }
+
+ context 'Application settings allows local requests' do
+ before do
+ allow(ApplicationSetting)
+ .to receive(:current)
+ .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true))
+ end
+
+ it { expect(kubernetes.save).to be_truthy }
+ end
+ end
end
context 'when validates token' do
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index 41159348e04..72c6161424b 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -32,17 +32,56 @@ describe Issuable do
end
describe "Validation" do
- subject { build(:issue) }
+ context 'general validations' do
+ subject { build(:issue) }
- before do
- allow(InternalId).to receive(:generate_next).and_return(nil)
+ before do
+ allow(InternalId).to receive(:generate_next).and_return(nil)
+ end
+
+ it { is_expected.to validate_presence_of(:project) }
+ it { is_expected.to validate_presence_of(:iid) }
+ it { is_expected.to validate_presence_of(:author) }
+ it { is_expected.to validate_presence_of(:title) }
+ it { is_expected.to validate_length_of(:title).is_at_most(255) }
end
- it { is_expected.to validate_presence_of(:project) }
- it { is_expected.to validate_presence_of(:iid) }
- it { is_expected.to validate_presence_of(:author) }
- it { is_expected.to validate_presence_of(:title) }
- it { is_expected.to validate_length_of(:title).is_at_most(255) }
+ describe 'milestone' do
+ let(:project) { create(:project) }
+ let(:milestone_id) { create(:milestone, project: project).id }
+ let(:params) do
+ {
+ title: 'something',
+ project: project,
+ author: build(:user),
+ milestone_id: milestone_id
+ }
+ end
+
+ subject { issuable_class.new(params) }
+
+ context 'with correct params' do
+ it { is_expected.to be_valid }
+ end
+
+ context 'with empty string milestone' do
+ let(:milestone_id) { '' }
+
+ it { is_expected.to be_valid }
+ end
+
+ context 'with nil milestone id' do
+ let(:milestone_id) { nil }
+
+ it { is_expected.to be_valid }
+ end
+
+ context 'with a milestone id from another project' do
+ let(:milestone_id) { create(:milestone).id }
+
+ it { is_expected.to be_invalid }
+ end
+ end
end
describe "Scope" do
@@ -66,6 +105,48 @@ describe Issuable do
end
end
+ describe '#milestone_available?' do
+ let(:group) { create(:group) }
+ let(:project) { create(:project, group: group) }
+ let(:issue) { create(:issue, project: project) }
+
+ def build_issuable(milestone_id)
+ issuable_class.new(project: project, milestone_id: milestone_id)
+ end
+
+ it 'returns true with a milestone from the issue project' do
+ milestone = create(:milestone, project: project)
+
+ expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+ end
+
+ it 'returns true with a milestone from the issue project group' do
+ milestone = create(:milestone, group: group)
+
+ expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+ end
+
+ it 'returns true with a milestone from the the parent of the issue project group', :nested_groups do
+ parent = create(:group)
+ group.update(parent: parent)
+ milestone = create(:milestone, group: parent)
+
+ expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+ end
+
+ it 'returns false with a milestone from another project' do
+ milestone = create(:milestone)
+
+ expect(build_issuable(milestone.id).milestone_available?).to be_falsey
+ end
+
+ it 'returns false with a milestone from another group' do
+ milestone = create(:milestone, group: create(:group))
+
+ expect(build_issuable(milestone.id).milestone_available?).to be_falsey
+ end
+ end
+
describe ".search" do
let!(:searchable_issue) { create(:issue, title: "Searchable awesome issue") }
let!(:searchable_issue2) { create(:issue, title: 'Aw') }
diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb
index 87bf731340f..4647eecbdef 100644
--- a/spec/models/concerns/milestoneish_spec.rb
+++ b/spec/models/concerns/milestoneish_spec.rb
@@ -48,7 +48,7 @@ describe Milestone, 'Milestoneish' do
merge_request_2 = create(:labeled_merge_request, labels: [label_1], source_project: project, source_branch: 'branch_2', milestone: milestone)
merge_request_3 = create(:labeled_merge_request, labels: [label_3], source_project: project, source_branch: 'branch_3', milestone: milestone)
- merge_requests = milestone.sorted_merge_requests
+ merge_requests = milestone.sorted_merge_requests(member)
expect(merge_requests.first).to eq(merge_request_2)
expect(merge_requests.second).to eq(merge_request_1)
@@ -56,6 +56,51 @@ describe Milestone, 'Milestoneish' do
end
end
+ describe '#merge_requests_visible_to_user' do
+ let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
+
+ context 'when project is private' do
+ before do
+ project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+ end
+
+ it 'does not return any merge request for a non member' do
+ merge_requests = milestone.merge_requests_visible_to_user(non_member)
+ expect(merge_requests).to be_empty
+ end
+
+ it 'returns milestone merge requests for a member' do
+ merge_requests = milestone.merge_requests_visible_to_user(member)
+ expect(merge_requests).to contain_exactly(merge_request)
+ end
+ end
+
+ context 'when project is public' do
+ context 'when merge requests are available to anyone' do
+ it 'returns milestone merge requests for a non member' do
+ merge_requests = milestone.merge_requests_visible_to_user(non_member)
+ expect(merge_requests).to contain_exactly(merge_request)
+ end
+ end
+
+ context 'when merge requests are available to project members' do
+ before do
+ project.project_feature.update(merge_requests_access_level: ProjectFeature::PRIVATE)
+ end
+
+ it 'does not return any merge request for a non member' do
+ merge_requests = milestone.merge_requests_visible_to_user(non_member)
+ expect(merge_requests).to be_empty
+ end
+
+ it 'returns milestone merge requests for a member' do
+ merge_requests = milestone.merge_requests_visible_to_user(member)
+ expect(merge_requests).to contain_exactly(merge_request)
+ end
+ end
+ end
+ end
+
describe '#closed_items_count' do
it 'does not count confidential issues for non project members' do
expect(milestone.closed_items_count(non_member)).to eq 2
diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb
index 1bf0ecb98ad..b7291eebe64 100644
--- a/spec/models/issue/metrics_spec.rb
+++ b/spec/models/issue/metrics_spec.rb
@@ -9,7 +9,7 @@ describe Issue::Metrics do
context "milestones" do
it "records the first time an issue is associated with a milestone" do
time = Time.now
- Timecop.freeze(time) { subject.update(milestone: create(:milestone)) }
+ Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) }
metrics = subject.metrics
expect(metrics).to be_present
@@ -18,9 +18,9 @@ describe Issue::Metrics do
it "does not record the second time an issue is associated with a milestone" do
time = Time.now
- Timecop.freeze(time) { subject.update(milestone: create(:milestone)) }
+ Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) }
Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) }
- Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone)) }
+ Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone, project: project)) }
metrics = subject.metrics
expect(metrics).to be_present
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 1849d3bac12..e530e0302f5 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -3,6 +3,18 @@ require 'spec_helper'
describe MergeRequestDiff do
let(:diff_with_commits) { create(:merge_request).merge_request_diff }
+ describe 'validations' do
+ subject { diff_with_commits }
+
+ it 'checks sha format of base_commit_sha, head_commit_sha and start_commit_sha' do
+ subject.base_commit_sha = subject.head_commit_sha = subject.start_commit_sha = 'foobar'
+
+ expect(subject.valid?).to be false
+ expect(subject.errors.count).to eq 3
+ expect(subject.errors).to all(include('is not a valid SHA'))
+ end
+ end
+
describe 'create new record' do
subject { diff_with_commits }
@@ -78,7 +90,7 @@ describe MergeRequestDiff do
it 'returns persisted diffs if cannot compare with diff refs' do
expect(diff).to receive(:load_diffs).and_call_original
- diff.update!(head_commit_sha: 'invalid-sha')
+ diff.update!(head_commit_sha: Digest::SHA1.hexdigest(SecureRandom.hex))
diff.diffs.diff_files
end
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index af7e3d3a6c9..77b7042424c 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -182,7 +182,7 @@ describe Milestone do
describe '#total_items_count' do
before do
create :closed_issue, milestone: milestone, project: project
- create :merge_request, milestone: milestone
+ create :merge_request, milestone: milestone, source_project: project
end
it 'returns total count of issues and merge requests assigned to milestone' do
@@ -192,10 +192,10 @@ describe Milestone do
describe '#can_be_closed?' do
before do
- milestone = create :milestone
- create :closed_issue, milestone: milestone
+ milestone = create :milestone, project: project
+ create :closed_issue, milestone: milestone, project: project
- create :issue
+ create :issue, project: project
end
it 'returns true if milestone active and all nested issues closed' do
diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb
index b6cf4c72450..e9c7c94ad70 100644
--- a/spec/models/project_services/prometheus_service_spec.rb
+++ b/spec/models/project_services/prometheus_service_spec.rb
@@ -33,18 +33,38 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
describe 'Validations' do
context 'when manual_configuration is enabled' do
before do
- subject.manual_configuration = true
+ service.manual_configuration = true
end
- it { is_expected.to validate_presence_of(:api_url) }
+ it 'validates presence of api_url' do
+ expect(service).to validate_presence_of(:api_url)
+ end
end
context 'when manual configuration is disabled' do
before do
- subject.manual_configuration = false
+ service.manual_configuration = false
end
- it { is_expected.not_to validate_presence_of(:api_url) }
+ it 'does not validate presence of api_url' do
+ expect(service).not_to validate_presence_of(:api_url)
+ end
+ end
+
+ context 'when the api_url domain points to localhost or local network' do
+ let(:domain) { Addressable::URI.parse(service.api_url).hostname }
+
+ it 'cannot query' do
+ expect(service.can_query?).to be true
+
+ aggregate_failures do
+ ['127.0.0.1', '192.168.2.3'].each do |url|
+ allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([Addrinfo.tcp(url, 80)])
+
+ expect(service.can_query?).to be false
+ end
+ end
+ end
end
end
@@ -74,30 +94,35 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
end
describe '#prometheus_client' do
+ let(:api_url) { 'http://some_url' }
+
+ before do
+ service.active = true
+ service.api_url = api_url
+ service.manual_configuration = manual_configuration
+ end
+
context 'manual configuration is enabled' do
- let(:api_url) { 'http://some_url' }
+ let(:manual_configuration) { true }
- before do
- subject.active = true
- subject.manual_configuration = true
- subject.api_url = api_url
+ it 'returns rest client from api_url' do
+ expect(service.prometheus_client.url).to eq(api_url)
end
- it 'returns rest client from api_url' do
- expect(subject.prometheus_client.url).to eq(api_url)
+ it 'calls valid?' do
+ allow(service).to receive(:valid?).and_call_original
+
+ expect(service.prometheus_client).not_to be_nil
+
+ expect(service).to have_received(:valid?)
end
end
context 'manual configuration is disabled' do
- let(:api_url) { 'http://some_url' }
-
- before do
- subject.manual_configuration = false
- subject.api_url = api_url
- end
+ let(:manual_configuration) { false }
it 'no client provided' do
- expect(subject.prometheus_client).to be_nil
+ expect(service.prometheus_client).to be_nil
end
end
end
diff --git a/spec/policies/commit_policy_spec.rb b/spec/policies/commit_policy_spec.rb
new file mode 100644
index 00000000000..41f6fb08426
--- /dev/null
+++ b/spec/policies/commit_policy_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe CommitPolicy do
+ describe '#rules' do
+ let(:user) { create(:user) }
+ let(:commit) { project.repository.head_commit }
+ let(:policy) { described_class.new(user, commit) }
+
+ context 'when project is public' do
+ let(:project) { create(:project, :public, :repository) }
+
+ it 'can read commit and create a note' do
+ expect(policy).to be_allowed(:read_commit)
+ end
+
+ context 'when repository access level is private' do
+ let(:project) { create(:project, :public, :repository, :repository_private) }
+
+ it 'can not read commit and create a note' do
+ expect(policy).to be_disallowed(:read_commit)
+ end
+
+ context 'when the user is a project member' do
+ before do
+ project.add_developer(user)
+ end
+
+ it 'can read commit and create a note' do
+ expect(policy).to be_allowed(:read_commit)
+ end
+ end
+ end
+ end
+
+ context 'when project is private' do
+ let(:project) { create(:project, :private, :repository) }
+
+ it 'can not read commit and create a note' do
+ expect(policy).to be_disallowed(:read_commit)
+ end
+
+ context 'when the user is a project member' do
+ before do
+ project.add_developer(user)
+ end
+
+ it 'can read commit and create a note' do
+ expect(policy).to be_allowed(:read_commit)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb
index af6d6f084a9..0ad50c6f91f 100644
--- a/spec/policies/group_policy_spec.rb
+++ b/spec/policies/group_policy_spec.rb
@@ -74,6 +74,38 @@ describe GroupPolicy do
end
end
+ context 'with no user and public project' do
+ let(:project) { create(:project, :public) }
+ let(:user) { create(:user) }
+ let(:current_user) { nil }
+
+ before do
+ Projects::GroupLinks::CreateService.new(
+ project,
+ user,
+ link_group_access: ProjectGroupLink::DEVELOPER
+ ).execute(group)
+ end
+
+ it { expect_disallowed(:read_group) }
+ end
+
+ context 'with foreign user and public project' do
+ let(:project) { create(:project, :public) }
+ let(:user) { create(:user) }
+ let(:current_user) { create(:user) }
+
+ before do
+ Projects::GroupLinks::CreateService.new(
+ project,
+ user,
+ link_group_access: ProjectGroupLink::DEVELOPER
+ ).execute(group)
+ end
+
+ it { expect_disallowed(:read_group) }
+ end
+
context 'has projects' do
let(:current_user) { create(:user) }
let(:project) { create(:project, namespace: group) }
@@ -82,17 +114,13 @@ describe GroupPolicy do
project.add_developer(current_user)
end
- it do
- expect_allowed(:read_group, :read_list, :read_label)
- end
+ it { expect_allowed(:read_label, :read_list) }
context 'in subgroups', :nested_groups do
let(:subgroup) { create(:group, :private, parent: group) }
let(:project) { create(:project, namespace: subgroup) }
- it do
- expect_allowed(:read_group, :read_list, :read_label)
- end
+ it { expect_allowed(:read_label, :read_list) }
end
end
diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb
index 0e848c74659..4be7a0266d1 100644
--- a/spec/policies/note_policy_spec.rb
+++ b/spec/policies/note_policy_spec.rb
@@ -1,28 +1,15 @@
require 'spec_helper'
-describe NotePolicy, mdoels: true do
+describe NotePolicy do
describe '#rules' do
let(:user) { create(:user) }
let(:project) { create(:project, :public) }
let(:issue) { create(:issue, project: project) }
-
- def policies(noteable = nil)
- return @policies if @policies
-
- noteable ||= issue
- note = if noteable.is_a?(Commit)
- create(:note_on_commit, commit_id: noteable.id, author: user, project: project)
- else
- create(:note, noteable: noteable, author: user, project: project)
- end
-
- @policies = described_class.new(user, note)
- end
+ let(:noteable) { issue }
+ let(:policy) { described_class.new(user, note) }
+ let(:note) { create(:note, noteable: noteable, author: user, project: project) }
shared_examples_for 'a discussion with a private noteable' do
- let(:noteable) { issue }
- let(:policy) { policies(noteable) }
-
context 'when the note author can no longer see the noteable' do
it 'can not edit nor read the note' do
expect(policy).to be_disallowed(:admin_note)
@@ -46,12 +33,21 @@ describe NotePolicy, mdoels: true do
end
end
- context 'when the project is private' do
- let(:project) { create(:project, :private, :repository) }
+ context 'when the noteable is a commit' do
+ let(:commit) { project.repository.head_commit }
+ let(:note) { create(:note_on_commit, commit_id: commit.id, author: user, project: project) }
+
+ context 'when the project is private' do
+ let(:project) { create(:project, :private, :repository) }
+
+ it_behaves_like 'a discussion with a private noteable'
+ end
- context 'when the noteable is a commit' do
- it_behaves_like 'a discussion with a private noteable' do
- let(:noteable) { project.repository.head_commit }
+ context 'when the project is public' do
+ context 'when repository access level is private' do
+ let(:project) { create(:project, :public, :repository, :repository_private) }
+
+ it_behaves_like 'a discussion with a private noteable'
end
end
end
@@ -59,44 +55,44 @@ describe NotePolicy, mdoels: true do
context 'when the project is public' do
context 'when the note author is not a project member' do
it 'can edit a note' do
- expect(policies).to be_allowed(:admin_note)
- expect(policies).to be_allowed(:resolve_note)
- expect(policies).to be_allowed(:read_note)
+ expect(policy).to be_allowed(:admin_note)
+ expect(policy).to be_allowed(:resolve_note)
+ expect(policy).to be_allowed(:read_note)
end
end
context 'when the noteable is a project snippet' do
- it 'can edit note' do
- policies = policies(create(:project_snippet, :public, project: project))
+ let(:noteable) { create(:project_snippet, :public, project: project) }
- expect(policies).to be_allowed(:admin_note)
- expect(policies).to be_allowed(:resolve_note)
- expect(policies).to be_allowed(:read_note)
+ it 'can edit note' do
+ expect(policy).to be_allowed(:admin_note)
+ expect(policy).to be_allowed(:resolve_note)
+ expect(policy).to be_allowed(:read_note)
end
context 'when it is private' do
- it_behaves_like 'a discussion with a private noteable' do
- let(:noteable) { create(:project_snippet, :private, project: project) }
- end
+ let(:noteable) { create(:project_snippet, :private, project: project) }
+
+ it_behaves_like 'a discussion with a private noteable'
end
end
context 'when the noteable is a personal snippet' do
- it 'can edit note' do
- policies = policies(create(:personal_snippet, :public))
+ let(:noteable) { create(:personal_snippet, :public) }
- expect(policies).to be_allowed(:admin_note)
- expect(policies).to be_allowed(:resolve_note)
- expect(policies).to be_allowed(:read_note)
+ it 'can edit note' do
+ expect(policy).to be_allowed(:admin_note)
+ expect(policy).to be_allowed(:resolve_note)
+ expect(policy).to be_allowed(:read_note)
end
context 'when it is private' do
- it 'can not edit nor read the note' do
- policies = policies(create(:personal_snippet, :private))
+ let(:noteable) { create(:personal_snippet, :private) }
- expect(policies).to be_disallowed(:admin_note)
- expect(policies).to be_disallowed(:resolve_note)
- expect(policies).to be_disallowed(:read_note)
+ it 'can not edit nor read the note' do
+ expect(policy).to be_disallowed(:admin_note)
+ expect(policy).to be_disallowed(:resolve_note)
+ expect(policy).to be_disallowed(:read_note)
end
end
end
@@ -120,20 +116,20 @@ describe NotePolicy, mdoels: true do
end
it 'can edit a note' do
- expect(policies).to be_allowed(:admin_note)
- expect(policies).to be_allowed(:resolve_note)
- expect(policies).to be_allowed(:read_note)
+ expect(policy).to be_allowed(:admin_note)
+ expect(policy).to be_allowed(:resolve_note)
+ expect(policy).to be_allowed(:read_note)
end
end
context 'when the note author is not a project member' do
it 'can not edit a note' do
- expect(policies).to be_disallowed(:admin_note)
- expect(policies).to be_disallowed(:resolve_note)
+ expect(policy).to be_disallowed(:admin_note)
+ expect(policy).to be_disallowed(:resolve_note)
end
it 'can read a note' do
- expect(policies).to be_allowed(:read_note)
+ expect(policy).to be_allowed(:read_note)
end
end
end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 997bdc82af6..47491f708e9 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -131,22 +131,26 @@ describe ProjectPolicy do
subject { described_class.new(owner, project) }
context 'when the feature is disabled' do
- it 'does not include the issues permissions' do
+ before do
project.issues_enabled = false
project.save!
+ end
+ it 'does not include the issues permissions' do
expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue
end
- end
- context 'when the feature is disabled and external tracker configured' do
- it 'does not include the issues permissions' do
- create(:jira_service, project: project)
+ it 'disables boards and lists permissions' do
+ expect_disallowed :read_board, :create_board, :update_board, :admin_board
+ expect_disallowed :read_list, :create_list, :update_list, :admin_list
+ end
- project.issues_enabled = false
- project.save!
+ context 'when external tracker configured' do
+ it 'does not include the issues permissions' do
+ create(:jira_service, project: project)
- expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue
+ expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue
+ end
end
end
end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 066f1d6862a..a132b85b878 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -1430,8 +1430,8 @@ describe API::Commits do
end
describe 'GET /projects/:id/repository/commits/:sha/merge_requests' do
- let!(:project) { create(:project, :repository, :private) }
- let!(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') }
+ let(:project) { create(:project, :repository, :private) }
+ let(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') }
let(:commit) { merged_mr.merge_request_diff.commits.last }
it 'returns the correct merge request' do
@@ -1456,6 +1456,17 @@ describe API::Commits do
expect(response).to have_gitlab_http_status(404)
end
+
+ context 'public project' do
+ let(:project) { create(:project, :repository, :public, :merge_requests_private) }
+ let(:non_member) { create(:user) }
+
+ it 'responds 403 when only members are allowed to read merge requests' do
+ get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", non_member)
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
end
describe 'GET /projects/:id/repository/commits/:sha/signature' do
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 01bab2a1361..f35dabf5d0f 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -49,7 +49,7 @@ describe API::Issues do
create(:label, title: 'label', color: '#FFAABB', project: project)
end
let!(:label_link) { create(:label_link, label: label, target: issue) }
- set(:milestone) { create(:milestone, title: '1.0.0', project: project) }
+ let(:milestone) { create(:milestone, title: '1.0.0', project: project) }
set(:empty_milestone) do
create(:milestone, title: '2.0.0', project: project)
end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 856fe1bbe89..60d9d7fed13 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -136,6 +136,7 @@ describe API::Projects do
end
let!(:public_project) { create(:project, :public, name: 'public_project') }
+
before do
project
project2
@@ -968,8 +969,16 @@ describe API::Projects do
describe 'GET /projects/:id' do
context 'when unauthenticated' do
- it 'returns the public projects' do
- public_project = create(:project, :public)
+ it 'does not return private projects' do
+ private_project = create(:project, :private)
+
+ get api("/projects/#{private_project.id}")
+
+ expect(response).to have_gitlab_http_status(404)
+ end
+
+ it 'returns public projects' do
+ public_project = create(:project, :repository, :public)
get api("/projects/#{public_project.id}")
@@ -977,8 +986,34 @@ describe API::Projects do
expect(json_response['id']).to eq(public_project.id)
expect(json_response['description']).to eq(public_project.description)
expect(json_response['default_branch']).to eq(public_project.default_branch)
+ expect(json_response['ci_config_path']).to eq(public_project.ci_config_path)
expect(json_response.keys).not_to include('permissions')
end
+
+ context 'and the project has a private repository' do
+ let(:project) { create(:project, :repository, :public, :repository_private) }
+ let(:protected_attributes) { %w(default_branch ci_config_path) }
+
+ it 'hides protected attributes of private repositories if user is not a member' do
+ get api("/projects/#{project.id}", user)
+
+ expect(response).to have_gitlab_http_status(200)
+ protected_attributes.each do |attribute|
+ expect(json_response.keys).not_to include(attribute)
+ end
+ end
+
+ it 'exposes protected attributes of private repositories if user is a member' do
+ project.add_developer(user)
+
+ get api("/projects/#{project.id}", user)
+
+ expect(response).to have_gitlab_http_status(200)
+ protected_attributes.each do |attribute|
+ expect(json_response.keys).to include(attribute)
+ end
+ end
+ end
end
context 'when authenticated' do
@@ -1130,6 +1165,26 @@ describe API::Projects do
expect(json_response).to include 'statistics'
end
+ context "and the project has a private repository" do
+ let(:project) { create(:project, :public, :repository, :repository_private) }
+
+ it "does not include statistics if user is not a member" do
+ get api("/projects/#{project.id}", user), params: { statistics: true }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response).not_to include 'statistics'
+ end
+
+ it "includes statistics if user is a member" do
+ project.add_developer(user)
+
+ get api("/projects/#{project.id}", user), params: { statistics: true }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response).to include 'statistics'
+ end
+ end
+
it "includes import_error if user can admin project" do
get api("/projects/#{project.id}", user)
@@ -1510,6 +1565,9 @@ describe API::Projects do
describe "POST /projects/:id/share" do
let(:group) { create(:group) }
+ before do
+ group.add_developer(user)
+ end
it "shares project with group" do
expires_at = 10.days.from_now.to_date
@@ -1560,6 +1618,15 @@ describe API::Projects do
expect(response).to have_gitlab_http_status(400)
expect(json_response['error']).to eq 'group_access does not have a valid value'
end
+
+ it "returns a 409 error when link is not saved" do
+ allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute)
+ .and_return({ status: :error, http_status: 409, message: 'error' })
+
+ post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: Gitlab::Access::DEVELOPER }
+
+ expect(response).to have_gitlab_http_status(409)
+ end
end
describe 'DELETE /projects/:id/share/:group_id' do
diff --git a/spec/requests/api/release/links_spec.rb b/spec/requests/api/release/links_spec.rb
index ba948e37e2f..3a59052bb29 100644
--- a/spec/requests/api/release/links_spec.rb
+++ b/spec/requests/api/release/links_spec.rb
@@ -73,6 +73,22 @@ describe API::Release::Links do
expect(response).to have_gitlab_http_status(:ok)
end
end
+
+ context 'when project is public and the repository is private' do
+ let(:project) { create(:project, :repository, :public, :repository_private) }
+
+ it_behaves_like '403 response' do
+ let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) }
+ end
+
+ context 'when the release does not exists' do
+ let!(:release) { }
+
+ it_behaves_like '403 response' do
+ let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) }
+ end
+ end
+ end
end
end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 5b625fd47be..bfa178f5cae 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -104,6 +104,70 @@ describe 'Git HTTP requests' do
end
end
+ shared_examples_for 'project path without .git suffix' do
+ context "GET info/refs" do
+ let(:path) { "/#{project_path}/info/refs" }
+
+ context "when no params are added" do
+ before do
+ get path
+ end
+
+ it "redirects to the .git suffix version" do
+ expect(response).to redirect_to("/#{project_path}.git/info/refs")
+ end
+ end
+
+ context "when the upload-pack service is requested" do
+ let(:params) { { service: 'git-upload-pack' } }
+
+ before do
+ get path, params: params
+ end
+
+ it "redirects to the .git suffix version" do
+ expect(response).to redirect_to("/#{project_path}.git/info/refs?service=#{params[:service]}")
+ end
+ end
+
+ context "when the receive-pack service is requested" do
+ let(:params) { { service: 'git-receive-pack' } }
+
+ before do
+ get path, params: params
+ end
+
+ it "redirects to the .git suffix version" do
+ expect(response).to redirect_to("/#{project_path}.git/info/refs?service=#{params[:service]}")
+ end
+ end
+
+ context "when the params are anything else" do
+ let(:params) { { service: 'git-implode-pack' } }
+
+ before do
+ get path, params: params
+ end
+
+ it "redirects to the sign-in page" do
+ expect(response).to redirect_to(new_user_session_path)
+ end
+ end
+ end
+
+ context "POST git-upload-pack" do
+ it "fails to find a route" do
+ expect { clone_post(project_path) }.to raise_error(ActionController::RoutingError)
+ end
+ end
+
+ context "POST git-receive-pack" do
+ it "fails to find a route" do
+ expect { push_post(project_path) }.to raise_error(ActionController::RoutingError)
+ end
+ end
+ end
+
describe "User with no identities" do
let(:user) { create(:user) }
@@ -143,6 +207,10 @@ describe 'Git HTTP requests' do
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
+
+ it_behaves_like 'project path without .git suffix' do
+ let(:project_path) { "#{user.namespace.path}/project.git-project" }
+ end
end
end
@@ -706,70 +774,8 @@ describe 'Git HTTP requests' do
end
end
- context "when the project path doesn't end in .git" do
- let(:project) { create(:project, :repository, :public, path: 'project.git-project') }
-
- context "GET info/refs" do
- let(:path) { "/#{project.full_path}/info/refs" }
-
- context "when no params are added" do
- before do
- get path
- end
-
- it "redirects to the .git suffix version" do
- expect(response).to redirect_to("/#{project.full_path}.git/info/refs")
- end
- end
-
- context "when the upload-pack service is requested" do
- let(:params) { { service: 'git-upload-pack' } }
-
- before do
- get path, params: params
- end
-
- it "redirects to the .git suffix version" do
- expect(response).to redirect_to("/#{project.full_path}.git/info/refs?service=#{params[:service]}")
- end
- end
-
- context "when the receive-pack service is requested" do
- let(:params) { { service: 'git-receive-pack' } }
-
- before do
- get path, params: params
- end
-
- it "redirects to the .git suffix version" do
- expect(response).to redirect_to("/#{project.full_path}.git/info/refs?service=#{params[:service]}")
- end
- end
-
- context "when the params are anything else" do
- let(:params) { { service: 'git-implode-pack' } }
-
- before do
- get path, params: params
- end
-
- it "redirects to the sign-in page" do
- expect(response).to redirect_to(new_user_session_path)
- end
- end
- end
-
- context "POST git-upload-pack" do
- it "fails to find a route" do
- expect { clone_post(project.full_path) }.to raise_error(ActionController::RoutingError)
- end
- end
-
- context "POST git-receive-pack" do
- it "fails to find a route" do
- expect { push_post(project.full_path) }.to raise_error(ActionController::RoutingError)
- end
- end
+ it_behaves_like 'project path without .git suffix' do
+ let(:project_path) { create(:project, :repository, :public, path: 'project.git-project').full_path }
end
context "retrieving an info/refs file" do
diff --git a/spec/services/issuable/common_system_notes_service_spec.rb b/spec/services/issuable/common_system_notes_service_spec.rb
index fa5d5ebac5c..0edc9016c96 100644
--- a/spec/services/issuable/common_system_notes_service_spec.rb
+++ b/spec/services/issuable/common_system_notes_service_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
describe Issuable::CommonSystemNotesService do
let(:user) { create(:user) }
let(:project) { create(:project) }
- let(:issuable) { create(:issue) }
+ let(:issuable) { create(:issue, project: project) }
context 'on issuable update' do
it_behaves_like 'system note creation', { title: 'New title' }, 'changed title'
@@ -70,7 +70,7 @@ describe Issuable::CommonSystemNotesService do
end
context 'on issuable create' do
- let(:issuable) { build(:issue) }
+ let(:issuable) { build(:issue, project: project) }
subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) }
diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb
index 248e7d5a389..86e58fe06b9 100644
--- a/spec/services/issues/build_service_spec.rb
+++ b/spec/services/issues/build_service_spec.rb
@@ -8,29 +8,29 @@ describe Issues::BuildService do
project.add_developer(user)
end
+ def build_issue(issue_params = {})
+ described_class.new(project, user, issue_params).execute
+ end
+
context 'for a single discussion' do
describe '#execute' do
let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) }
let(:discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "Almost done").to_discussion }
- let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) }
- it 'references the noteable title in the issue title' do
- issue = service.execute
+ subject { build_issue(merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) }
- expect(issue.title).to include('Hello world')
+ it 'references the noteable title in the issue title' do
+ expect(subject.title).to include('Hello world')
end
it 'adds the note content to the description' do
- issue = service.execute
-
- expect(issue.description).to include('Almost done')
+ expect(subject.description).to include('Almost done')
end
end
end
context 'for discussions in a merge request' do
let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
- let(:issue) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute }
describe '#items_for_discussions' do
it 'has an item for each discussion' do
@@ -66,28 +66,30 @@ describe Issues::BuildService do
end
describe '#execute' do
- it 'has the merge request reference in the title' do
- expect(issue.title).to include(merge_request.title)
- end
+ let(:base_params) { { merge_request_to_resolve_discussions_of: merge_request.iid } }
- it 'has the reference of the merge request in the description' do
- expect(issue.description).to include(merge_request.to_reference)
+ context 'without additional params' do
+ subject { build_issue(base_params) }
+
+ it 'has the merge request reference in the title' do
+ expect(subject.title).to include(merge_request.title)
+ end
+
+ it 'has the reference of the merge request in the description' do
+ expect(subject.description).to include(merge_request.to_reference)
+ end
end
- it 'does not assign title when a title was given' do
- issue = described_class.new(project, user,
- merge_request_to_resolve_discussions_of: merge_request,
- title: 'What an issue').execute
+ it 'uses provided title if title param given' do
+ issue = build_issue(base_params.merge(title: 'What an issue'))
expect(issue.title).to eq('What an issue')
end
- it 'does not assign description when a description was given' do
- issue = described_class.new(project, user,
- merge_request_to_resolve_discussions_of: merge_request,
- description: 'Fix at your earliest conveignance').execute
+ it 'uses provided description if description param given' do
+ issue = build_issue(base_params.merge(description: 'Fix at your earliest convenience'))
- expect(issue.description).to eq('Fix at your earliest conveignance')
+ expect(issue.description).to eq('Fix at your earliest convenience')
end
describe 'with multiple discussions' do
@@ -96,20 +98,20 @@ describe Issues::BuildService do
it 'mentions all the authors in the description' do
authors = merge_request.resolvable_discussions.map(&:author)
- expect(issue.description).to include(*authors.map(&:to_reference))
+ expect(build_issue(base_params).description).to include(*authors.map(&:to_reference))
end
it 'has a link for each unresolved discussion in the description' do
notes = merge_request.resolvable_discussions.map(&:first_note)
links = notes.map { |note| Gitlab::UrlBuilder.build(note) }
- expect(issue.description).to include(*links)
+ expect(build_issue(base_params).description).to include(*links)
end
it 'mentions additional notes' do
create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, in_reply_to: diff_note)
- expect(issue.description).to include('(+2 comments)')
+ expect(build_issue(base_params).description).to include('(+2 comments)')
end
end
end
@@ -120,7 +122,7 @@ describe Issues::BuildService do
describe '#execute' do
it 'mentions the merge request in the description' do
- issue = described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute
+ issue = build_issue(merge_request_to_resolve_discussions_of: merge_request.iid)
expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}")
end
@@ -128,20 +130,18 @@ describe Issues::BuildService do
end
describe '#execute' do
- let(:milestone) { create(:milestone, project: project) }
-
it 'builds a new issues with given params' do
- issue = described_class.new(
- project,
- user,
- title: 'Issue #1',
- description: 'Issue description',
- milestone_id: milestone.id
- ).execute
-
- expect(issue.title).to eq('Issue #1')
- expect(issue.description).to eq('Issue description')
+ milestone = create(:milestone, project: project)
+ issue = build_issue(milestone_id: milestone.id)
+
expect(issue.milestone).to eq(milestone)
end
+
+ it 'sets milestone to nil if it is not available for the project' do
+ milestone = create(:milestone, project: create(:project))
+ issue = build_issue(milestone_id: milestone.id)
+
+ expect(issue.milestone).to be_nil
+ end
end
end
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 931e47d3a77..f1684209729 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -356,7 +356,7 @@ describe Issues::UpdateService, :mailer do
it_behaves_like 'system notes for milestones'
it 'sends notifications for subscribers of changed milestone' do
- issue.milestone = create(:milestone)
+ issue.milestone = create(:milestone, project: project)
issue.save
@@ -380,7 +380,7 @@ describe Issues::UpdateService, :mailer do
end
it 'marks todos as done' do
- update_issue(milestone: create(:milestone))
+ update_issue(milestone: create(:milestone, project: project))
expect(todo.reload.done?).to eq true
end
@@ -389,7 +389,7 @@ describe Issues::UpdateService, :mailer do
it 'sends notifications for subscribers of changed milestone' do
perform_enqueued_jobs do
- update_issue(milestone: create(:milestone))
+ update_issue(milestone: create(:milestone, project: project))
end
should_email(subscriber)
diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb
index 536d0d345a4..057e8137a4e 100644
--- a/spec/services/merge_requests/build_service_spec.rb
+++ b/spec/services/merge_requests/build_service_spec.rb
@@ -229,6 +229,15 @@ describe MergeRequests::BuildService do
end
end
end
+
+ context 'when a milestone is from another project' do
+ let(:milestone) { create(:milestone, project: create(:project)) }
+ let(:milestone_id) { milestone.id }
+
+ it 'sets milestone to nil' do
+ expect(merge_request.milestone).to be_nil
+ end
+ end
end
end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 20580bf14b9..8e367db031c 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -328,7 +328,7 @@ describe MergeRequests::UpdateService, :mailer do
it_behaves_like 'system notes for milestones'
it 'sends notifications for subscribers of changed milestone' do
- merge_request.milestone = create(:milestone)
+ merge_request.milestone = create(:milestone, project: project)
merge_request.save
@@ -352,7 +352,7 @@ describe MergeRequests::UpdateService, :mailer do
end
it 'marks pending todos as done' do
- update_merge_request({ milestone: create(:milestone) })
+ update_merge_request({ milestone: create(:milestone, project: project) })
expect(pending_todo.reload).to be_done
end
@@ -361,7 +361,7 @@ describe MergeRequests::UpdateService, :mailer do
it 'sends notifications for subscribers of changed milestone' do
perform_enqueued_jobs do
- update_merge_request(milestone: create(:milestone))
+ update_merge_request(milestone: create(:milestone, project: project))
end
should_email(subscriber)
diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb
index ffb270d277e..68fd82b4cbe 100644
--- a/spec/services/projects/group_links/create_service_spec.rb
+++ b/spec/services/projects/group_links/create_service_spec.rb
@@ -12,6 +12,10 @@ describe Projects::GroupLinks::CreateService, '#execute' do
end
let(:subject) { described_class.new(project, user, opts) }
+ before do
+ group.add_developer(user)
+ end
+
it 'adds group to project' do
expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1)
end
@@ -19,4 +23,8 @@ describe Projects::GroupLinks::CreateService, '#execute' do
it 'returns false if group is blank' do
expect { subject.execute(nil) }.not_to change { project.project_group_links.count }
end
+
+ it 'returns error if user is not allowed to share with a group' do
+ expect { subject.execute(create :group) }.not_to change { project.project_group_links.count }
+ end
end
diff --git a/spec/support/helpers/file_mover_helpers.rb b/spec/support/helpers/file_mover_helpers.rb
new file mode 100644
index 00000000000..1ba7cc03354
--- /dev/null
+++ b/spec/support/helpers/file_mover_helpers.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module FileMoverHelpers
+ def stub_file_mover(file_path, stub_real_path: nil)
+ file_name = File.basename(file_path)
+ allow(Pathname).to receive(:new).and_call_original
+
+ expect_next_instance_of(Pathname, a_string_including(file_name)) do |pathname|
+ allow(pathname).to receive(:realpath) { stub_real_path || pathname.cleanpath }
+ end
+ end
+end
diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb
index 3fee6872498..4a0cf62a661 100644
--- a/spec/support/helpers/login_helpers.rb
+++ b/spec/support/helpers/login_helpers.rb
@@ -47,7 +47,7 @@ module LoginHelpers
end
def gitlab_sign_in_via(provider, user, uid, saml_response = nil)
- mock_auth_hash(provider, uid, user.email, saml_response)
+ mock_auth_hash_with_saml_xml(provider, uid, user.email, saml_response)
visit new_user_session_path
click_link provider
end
@@ -87,7 +87,12 @@ module LoginHelpers
click_link "oauth-login-#{provider}"
end
- def mock_auth_hash(provider, uid, email, saml_response = nil)
+ def mock_auth_hash_with_saml_xml(provider, uid, email, saml_response)
+ response_object = { document: saml_xml(saml_response) }
+ mock_auth_hash(provider, uid, email, response_object: response_object)
+ end
+
+ def mock_auth_hash(provider, uid, email, response_object: nil)
# The mock_auth configuration allows you to set per-provider (or default)
# authentication hashes to return during integration testing.
OmniAuth.config.mock_auth[provider.to_sym] = OmniAuth::AuthHash.new({
@@ -110,9 +115,7 @@ module LoginHelpers
image: 'mock_user_thumbnail_url'
}
},
- response_object: {
- document: saml_xml(saml_response)
- }
+ response_object: response_object
}
})
Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym]
diff --git a/spec/support/shared_examples/issuable_shared_examples.rb b/spec/support/shared_examples/issuable_shared_examples.rb
index c3d40c5b231..d97b21f71cd 100644
--- a/spec/support/shared_examples/issuable_shared_examples.rb
+++ b/spec/support/shared_examples/issuable_shared_examples.rb
@@ -31,7 +31,7 @@ shared_examples 'system notes for milestones' do
context 'project milestones' do
it 'creates a system note' do
expect do
- update_issuable(milestone: create(:milestone))
+ update_issuable(milestone: create(:milestone, project: project))
end.to change { Note.system.count }.by(1)
end
end
diff --git a/spec/support/shared_examples/requests/api/discussions.rb b/spec/support/shared_examples/requests/api/discussions.rb
index e44da4faa5a..eff8e401bad 100644
--- a/spec/support/shared_examples/requests/api/discussions.rb
+++ b/spec/support/shared_examples/requests/api/discussions.rb
@@ -86,6 +86,37 @@ shared_examples 'discussions API' do |parent_type, noteable_type, id_name|
expect(response).to have_gitlab_http_status(404)
end
end
+
+ context 'when a project is public with private repo access' do
+ let!(:parent) { create(:project, :public, :repository, :repository_private, :snippets_private) }
+ let!(:user_without_access) { create(:user) }
+
+ context 'when user is not a team member of private repo' do
+ before do
+ project.team.truncate
+ end
+
+ context "creating a new note" do
+ before do
+ post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user_without_access), params: { body: 'hi!' }
+ end
+
+ it 'raises 404 error' do
+ expect(response).to have_gitlab_http_status(404)
+ end
+ end
+
+ context "fetching a discussion" do
+ before do
+ get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{note.discussion_id}", user_without_access)
+ end
+
+ it 'raises 404 error' do
+ expect(response).to have_gitlab_http_status(404)
+ end
+ end
+ end
+ end
end
describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do
diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb
index de29d0c943f..e474a714b10 100644
--- a/spec/uploaders/file_mover_spec.rb
+++ b/spec/uploaders/file_mover_spec.rb
@@ -1,8 +1,9 @@
require 'spec_helper'
describe FileMover do
+ include FileMoverHelpers
+
let(:filename) { 'banana_sample.gif' }
- let(:file) { fixture_file_upload(File.join('spec', 'fixtures', filename)) }
let(:temp_file_path) { File.join('uploads/-/system/temp', 'secret55', filename) }
let(:temp_description) do
@@ -12,7 +13,7 @@ describe FileMover do
let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, 'secret55', filename) }
let(:snippet) { create(:personal_snippet, description: temp_description) }
- subject { described_class.new(file_path, snippet).execute }
+ subject { described_class.new(temp_file_path, snippet).execute }
describe '#execute' do
before do
@@ -20,6 +21,8 @@ describe FileMover do
expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path))
allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true)
allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10)
+
+ stub_file_mover(temp_file_path)
end
context 'when move and field update successful' do
@@ -66,4 +69,30 @@ describe FileMover do
end
end
end
+
+ context 'security' do
+ context 'when relative path is involved' do
+ let(:temp_file_path) { File.join('uploads/-/system/temp', '..', 'another_subdir_of_temp') }
+
+ it 'does not trigger move if path is outside designated directory' do
+ stub_file_mover('uploads/-/system/another_subdir_of_temp')
+ expect(FileUtils).not_to receive(:move)
+
+ subject
+
+ expect(snippet.reload.description).to eq(temp_description)
+ end
+ end
+
+ context 'when symlink is involved' do
+ it 'does not trigger move if path is outside designated directory' do
+ stub_file_mover(temp_file_path, stub_real_path: Pathname('/etc'))
+ expect(FileUtils).not_to receive(:move)
+
+ subject
+
+ expect(snippet.reload.description).to eq(temp_description)
+ end
+ end
+ end
end
diff --git a/spec/validators/sha_validator_spec.rb b/spec/validators/sha_validator_spec.rb
new file mode 100644
index 00000000000..dc1539cf318
--- /dev/null
+++ b/spec/validators/sha_validator_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ShaValidator do
+ let(:validator) { described_class.new(attributes: [:base_commit_sha]) }
+ let(:merge_diff) { build(:merge_request_diff) }
+
+ subject { validator.validate_each(merge_diff, :base_commit_sha, value) }
+
+ context 'with empty value' do
+ let(:value) { nil }
+
+ it 'does not add any error if value is empty' do
+ subject
+
+ expect(merge_diff.errors).to be_empty
+ end
+ end
+
+ context 'with valid sha' do
+ let(:value) { Digest::SHA1.hexdigest(SecureRandom.hex) }
+
+ it 'does not add any error if value is empty' do
+ subject
+
+ expect(merge_diff.errors).to be_empty
+ end
+ end
+
+ context 'with invalid sha' do
+ let(:value) { 'foo' }
+
+ it 'adds error to the record' do
+ expect(merge_diff.errors).to be_empty
+
+ subject
+
+ expect(merge_diff.errors).not_to be_empty
+ end
+ end
+end
diff --git a/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb b/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb
index a13a3046f55..d20d926f5a0 100644
--- a/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb
+++ b/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb
@@ -18,7 +18,7 @@ describe UpdateHeadPipelineForMergeRequestWorker do
context 'when merge request sha does not equal pipeline sha' do
before do
- merge_request.merge_request_diff.update(head_commit_sha: 'different_sha')
+ merge_request.merge_request_diff.update(head_commit_sha: Digest::SHA1.hexdigest(SecureRandom.hex))
end
it 'does not update head pipeline' do