diff options
91 files changed, 1139 insertions, 392 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 9084fa2f716..524cb55242b 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.1.0 +1.1.1 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 4122521804f..9fe9ff9d996 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -7.0.0
\ No newline at end of file +7.0.1 diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index 56b1896e9f1..4abea532d56 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -25,6 +25,9 @@ export default function renderMermaid($els) { }, // mermaidAPI options theme: 'neutral', + flowchart: { + htmlLabels: false, + }, }); $els.each((i, el) => { diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ceea5f0cc26..26195560efb 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -12,9 +12,9 @@ class ApplicationController < ActionController::Base include WorkhorseHelper include EnforcesTwoFactorAuthentication include WithPerformanceBar + include SessionlessAuthentication include InvalidUTF8ErrorHandler - before_action :authenticate_sessionless_user! before_action :authenticate_user! before_action :enforce_terms!, if: :should_enforce_terms? before_action :validate_user_service_ticket! @@ -151,13 +151,6 @@ class ApplicationController < ActionController::Base end end - # This filter handles personal access tokens, and atom requests with rss tokens - def authenticate_sessionless_user! - user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user - - sessionless_sign_in(user) if user - end - def log_exception(exception) Raven.capture_exception(exception) if sentry_enabled? @@ -424,25 +417,11 @@ class ApplicationController < ActionController::Base Gitlab::I18n.with_user_locale(current_user, &block) end - def sessionless_sign_in(user) - if user && can?(user, :log_in) - # Notice we are passing store false, so the user is not - # actually stored in the session and a token is needed - # for every request. If you want the token to work as a - # sign in token, you can simply remove store: false. - sign_in(user, store: false, message: :sessionless_sign_in) - end - end - def set_page_title_header # Per https://tools.ietf.org/html/rfc5987, headers need to be ISO-8859-1, not UTF-8 response.headers['Page-Title'] = URI.escape(page_title('GitLab')) end - def sessionless_user? - current_user && !session.keys.include?('warden.user.user.key') - end - def peek_request? request.path.start_with?('/-/peek') end diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index 3a45d6205ab..18efff37552 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -6,6 +6,7 @@ module NotesActions extend ActiveSupport::Concern included do + prepend_before_action :normalize_create_params, only: [:create] before_action :set_polling_interval_header, only: [:index] before_action :require_noteable!, only: [:index, :create] before_action :authorize_admin_note!, only: [:update, :destroy] @@ -236,6 +237,15 @@ module NotesActions DiscussionSerializer.new(project: project, noteable: noteable, current_user: current_user, note_entity: ProjectNoteEntity) end + # Avoids checking permissions in the wrong object - this ensures that the object we checked permissions for + # is the object we're actually creating a note in. + def normalize_create_params + params[:note].try do |note| + note[:noteable_id] = params[:target_id] + note[:noteable_type] = params[:target_type].classify + end + end + def note_project strong_memoize(:note_project) do next nil unless project diff --git a/app/controllers/concerns/sessionless_authentication.rb b/app/controllers/concerns/sessionless_authentication.rb new file mode 100644 index 00000000000..590eefc6dab --- /dev/null +++ b/app/controllers/concerns/sessionless_authentication.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# == SessionlessAuthentication +# +# Controller concern to handle PAT and RSS token authentication methods +# +module SessionlessAuthentication + # This filter handles personal access tokens, and atom requests with rss tokens + def authenticate_sessionless_user!(request_format) + user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user(request_format) + + sessionless_sign_in(user) if user + end + + def sessionless_user? + current_user && !session.keys.include?('warden.user.user.key') + end + + def sessionless_sign_in(user) + if user && can?(user, :log_in) + # Notice we are passing store false, so the user is not + # actually stored in the session and a token is needed + # for every request. If you want the token to work as a + # sign in token, you can simply remove store: false. + sign_in(user, store: false, message: :sessionless_sign_in) + end + end +end diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index e9686ed8d06..57e612d89d3 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -4,6 +4,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController include ParamsBackwardCompatibility include RendersMemberAccess + prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) } before_action :set_non_archived_param before_action :default_sorting skip_cross_project_access_check :index, :starred diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index b82caf30a91..3fa582cf25b 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -4,6 +4,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController include ActionView::Helpers::NumberHelper before_action :authorize_read_project!, only: :index + before_action :authorize_read_group!, only: :index before_action :find_todos, only: [:index, :destroy_all] def index @@ -60,6 +61,15 @@ class Dashboard::TodosController < Dashboard::ApplicationController end end + def authorize_read_group! + group_id = params[:group_id] + + if group_id.present? + group = Group.find(group_id) + render_404 unless can?(current_user, :read_group, group) + end + end + def find_todos @todos ||= TodosFinder.new(current_user, todo_params).execute end diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index c032fb2efb5..c149e84b06e 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -11,6 +11,9 @@ class DashboardController < Dashboard::ApplicationController :label_name ].freeze + prepend_before_action(only: [:issues]) { authenticate_sessionless_user!(:rss) } + prepend_before_action(only: [:issues_calendar]) { authenticate_sessionless_user!(:ics) } + before_action :event_filter, only: :activity before_action :projects, only: [:issues, :merge_requests] before_action :set_show_full_reference, only: [:issues, :merge_requests] diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index a1ec144410b..6ea4758ec32 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -3,6 +3,7 @@ class GraphqlController < ApplicationController # Unauthenticated users have access to the API for public data skip_before_action :authenticate_user! + prepend_before_action(only: [:execute]) { authenticate_sessionless_user!(:api) } before_action :check_graphql_feature_flag! diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 062c8c4e9e1..c5d8ac2ed77 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -9,6 +9,9 @@ class GroupsController < Groups::ApplicationController respond_to :html + prepend_before_action(only: [:show, :issues]) { authenticate_sessionless_user!(:rss) } + prepend_before_action(only: [:issues_calendar]) { authenticate_sessionless_user!(:ics) } + before_action :authenticate_user!, only: [:new, :create] before_action :group, except: [:index, :new, :create] diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index 84a2a461da7..8ba18aacc58 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -6,6 +6,7 @@ class Projects::CommitsController < Projects::ApplicationController include ExtractsPath include RendersCommits + prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) } before_action :whitelist_query_limiting, except: :commits_root before_action :require_non_empty_project before_action :assign_ref_vars, except: :commits_root diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index b06a6f3bb0d..f52402addfe 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -9,7 +9,10 @@ class Projects::IssuesController < Projects::ApplicationController include IssuesCalendar include SpammableActions - prepend_before_action :authenticate_user!, only: [:new] + prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) } + prepend_before_action(only: [:calendar]) { authenticate_sessionless_user!(:ics) } + prepend_before_action :authenticate_new_issue!, only: [:new] + prepend_before_action :store_uri, only: [:new, :show] before_action :whitelist_query_limiting, only: [:create, :create_merge_request, :move, :bulk_update] before_action :check_issues_available! @@ -217,16 +220,18 @@ class Projects::IssuesController < Projects::ApplicationController ] + [{ label_ids: [], assignee_ids: [] }] end - def authenticate_user! + def authenticate_new_issue! return if current_user notice = "Please sign in to create the new issue." + redirect_to new_user_session_path, notice: notice + end + + def store_uri if request.get? && !request.xhr? store_location_for :user, request.fullpath end - - redirect_to new_user_session_path, notice: notice end def serializer diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index 20998c97730..8e68014a30d 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -11,7 +11,10 @@ class Projects::MilestonesController < Projects::ApplicationController before_action :authorize_read_milestone! # Allow admin milestone - before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels, :promote] + before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels] + + # Allow to promote milestone + before_action :authorize_promote_milestone!, only: :promote respond_to :html @@ -78,7 +81,7 @@ class Projects::MilestonesController < Projects::ApplicationController def promote promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone) - flash[:notice] = flash_notice_for(promoted_milestone, project.group) + flash[:notice] = flash_notice_for(promoted_milestone, project_group) respond_to do |format| format.html do @@ -109,6 +112,12 @@ class Projects::MilestonesController < Projects::ApplicationController protected + def project_group + strong_memoize(:project_group) do + project.group + end + end + def milestones strong_memoize(:milestones) do MilestonesFinder.new(search_params).execute @@ -125,13 +134,17 @@ class Projects::MilestonesController < Projects::ApplicationController return render_404 unless can?(current_user, :admin_milestone, @project) end + def authorize_promote_milestone! + return render_404 unless can?(current_user, :admin_milestone, project_group) + end + def milestone_params params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event) end def search_params - if request.format.json? && @project.group && can?(current_user, :read_group, @project.group) - groups = @project.group.self_and_ancestors_ids + if request.format.json? && project_group && can?(current_user, :read_group, project_group) + groups = project_group.self_and_ancestors_ids end params.permit(:state).merge(project_ids: @project.id, group_ids: groups) diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index c8442ff3592..2b28670a49b 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -3,6 +3,8 @@ class Projects::TagsController < Projects::ApplicationController include SortingHelper + prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) } + # Authorize before_action :require_non_empty_project before_action :authorize_download_code! diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index ee438e160f2..af8cedc4e5b 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -7,6 +7,8 @@ class ProjectsController < Projects::ApplicationController include PreviewMarkdown include SendFileUpload + prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) } + before_action :whitelist_query_limiting, only: [:create] before_action :authenticate_user!, except: [:index, :show, :activity, :refs] before_action :redirect_git_extension, only: [:show] diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5b70c69d7f4..8b040dc080e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -14,6 +14,7 @@ class UsersController < ApplicationController calendar_activities: true skip_before_action :authenticate_user! + prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) } before_action :user, except: [:exists] before_action :authorize_read_user_profile!, only: [:calendar, :calendar_activities, :groups, :projects, :contributed_projects, :snippets] diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb index 94a030d9d57..9666080092b 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/milestones_helper.rb @@ -2,6 +2,7 @@ module MilestonesHelper include EntityDateHelper + include Gitlab::Utils::StrongMemoize def milestones_filter_path(opts = {}) if @project @@ -243,4 +244,16 @@ module MilestonesHelper dashboard_milestone_path(milestone.safe_title, title: milestone.title) end end + + def can_admin_project_milestones? + strong_memoize(:can_admin_project_milestones) do + can?(current_user, :admin_milestone, @project) + end + end + + def can_admin_group_milestones? + strong_memoize(:can_admin_group_milestones) do + can?(current_user, :admin_milestone, @project.group) + end + end end diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index d3284e90568..1b3c1f9a8a9 100644 --- a/app/mailers/emails/notes.rb +++ b/app/mailers/emails/notes.rb @@ -26,7 +26,7 @@ module Emails mail_answer_note_thread(@merge_request, @note, note_thread_options(recipient_id)) end - def note_snippet_email(recipient_id, note_id) + def note_project_snippet_email(recipient_id, note_id) setup_note_mail(note_id, recipient_id) @snippet = @note.noteable diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb index 6e2adc76ec6..a8c9e54f00c 100644 --- a/app/models/concerns/cache_markdown_field.rb +++ b/app/models/concerns/cache_markdown_field.rb @@ -15,7 +15,7 @@ module CacheMarkdownField # Increment this number every time the renderer changes its output CACHE_REDCARPET_VERSION = 3 CACHE_COMMONMARK_VERSION_START = 10 - CACHE_COMMONMARK_VERSION = 11 + CACHE_COMMONMARK_VERSION = 12 # changes to these attributes cause the cache to be invalidates INVALIDATED_BY = %w[author project].freeze diff --git a/app/models/note.rb b/app/models/note.rb index 725c3d68c37..39725f00420 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -313,7 +313,7 @@ class Note < ActiveRecord::Base end def to_ability_name - for_personal_snippet? ? 'personal_snippet' : noteable_type.underscore + for_snippet? ? noteable.class.name.underscore : noteable_type.underscore end def can_be_discussion_note? diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 509e5b6089b..620efd3768c 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -72,7 +72,7 @@ class PrometheusService < MonitoringService end def prometheus_client - RestClient::Resource.new(api_url) if api_url && manual_configuration? && active? + RestClient::Resource.new(api_url, max_redirects: 0) if api_url && manual_configuration? && active? end def prometheus_installed? diff --git a/app/policies/commit_policy.rb b/app/policies/commit_policy.rb index 67e9bc12804..4d4f0ba9267 100644 --- a/app/policies/commit_policy.rb +++ b/app/policies/commit_policy.rb @@ -2,4 +2,6 @@ class CommitPolicy < BasePolicy delegate { @subject.project } + + rule { can?(:download_code) }.enable :read_commit end diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb index bbc2b48b856..f22843b6463 100644 --- a/app/policies/note_policy.rb +++ b/app/policies/note_policy.rb @@ -9,8 +9,17 @@ class NotePolicy < BasePolicy condition(:editable, scope: :subject) { @subject.editable? } + condition(:can_read_noteable) { can?(:"read_#{@subject.to_ability_name}") } + rule { ~editable }.prevent :admin_note + # If user can't read the issue/MR/etc then they should not be allowed to do anything to their own notes + rule { ~can_read_noteable }.policy do + prevent :read_note + prevent :admin_note + prevent :resolve_note + end + rule { is_author }.policy do enable :read_note enable :admin_note diff --git a/app/views/devise/mailer/email_changed.html.haml b/app/views/devise/mailer/email_changed.html.haml new file mode 100644 index 00000000000..5398430fdfd --- /dev/null +++ b/app/views/devise/mailer/email_changed.html.haml @@ -0,0 +1,12 @@ += email_default_heading("Hello, #{@resource.name}!") + +- if @resource.try(:unconfirmed_email?) + %p + We're contacting you to notify you that your email is being changed to #{@resource.reload.unconfirmed_email}. +- else + %p + We're contacting you to notify you that your email has been changed to #{@resource.email}. + +%p + If you did not initiate this change, please contact your administrator + immediately. diff --git a/app/views/devise/mailer/email_changed.text.erb b/app/views/devise/mailer/email_changed.text.erb new file mode 100644 index 00000000000..18137389e7b --- /dev/null +++ b/app/views/devise/mailer/email_changed.text.erb @@ -0,0 +1,10 @@ +Hello, <%= @resource.name %>! + +<% if @resource.try(:unconfirmed_email?) %> +We're contacting you to notify you that your email is being changed to <%= @resource.reload.unconfirmed_email %>. +<% else %> +We're contacting you to notify you that your email has been changed to <%= @resource.email %>. +<% end %> + +If you did not initiate this change, please contact your administrator +immediately. diff --git a/app/views/notify/note_snippet_email.html.haml b/app/views/notify/note_project_snippet_email.html.haml index 5e69f01a486..5e69f01a486 100644 --- a/app/views/notify/note_snippet_email.html.haml +++ b/app/views/notify/note_project_snippet_email.html.haml diff --git a/app/views/notify/note_snippet_email.text.erb b/app/views/notify/note_project_snippet_email.text.erb index 413d9e6e9ac..413d9e6e9ac 100644 --- a/app/views/notify/note_snippet_email.text.erb +++ b/app/views/notify/note_project_snippet_email.text.erb diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index c6789e32dbe..1a74b120c26 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -8,62 +8,50 @@ - ref = local_assigns.fetch(:ref) { merge_request&.source_branch } - link = commit_path(project, commit, merge_request: merge_request) -- cache_key = [project.full_path, - ref, - commit.id, - Gitlab::CurrentSettings.current_application_settings, - @path.presence, - current_controller?(:commits), - merge_request&.iid, - view_details, - commit.status(ref), - I18n.locale].compact - -= cache(cache_key, expires_in: 1.day) do - %li.commit.flex-row.js-toggle-container{ id: "commit-#{commit.short_id}" } - - .avatar-cell.d-none.d-sm-block - = author_avatar(commit, size: 36, has_tooltip: false) - - .commit-detail.flex-list - .commit-content.qa-commit-content - - if view_details && merge_request - = link_to commit.title, project_commit_path(project, commit.id, merge_request_iid: merge_request.iid), class: "commit-row-message item-title" - - else - = link_to_markdown_field(commit, :title, link, class: "commit-row-message item-title") - %span.commit-row-message.d-block.d-sm-none - · - = commit.short_id - - if commit.status(ref) - .d-block.d-sm-none - = render_commit_status(commit, ref: ref) - - if commit.description? - %button.text-expander.js-toggle-button - = sprite_icon('ellipsis_h', size: 12) +%li.commit.flex-row.js-toggle-container{ id: "commit-#{commit.short_id}" } + + .avatar-cell.d-none.d-sm-block + = author_avatar(commit, size: 36, has_tooltip: false) + + .commit-detail.flex-list + .commit-content.qa-commit-content + - if view_details && merge_request + = link_to commit.title, project_commit_path(project, commit.id, merge_request_iid: merge_request.iid), class: "commit-row-message item-title" + - else + = link_to_markdown_field(commit, :title, link, class: "commit-row-message item-title") + %span.commit-row-message.d-block.d-sm-none + · + = commit.short_id + - if commit.status(ref) + .d-block.d-sm-none + = render_commit_status(commit, ref: ref) + - if commit.description? + %button.text-expander.js-toggle-button + = sprite_icon('ellipsis_h', size: 12) - .committer - - commit_author_link = commit_author_link(commit, avatar: false, size: 24) - - commit_timeago = time_ago_with_tooltip(commit.authored_date, placement: 'bottom') - - commit_text = _('%{commit_author_link} authored %{commit_timeago}') % { commit_author_link: commit_author_link, commit_timeago: commit_timeago } - #{ commit_text.html_safe } + .committer + - commit_author_link = commit_author_link(commit, avatar: false, size: 24) + - commit_timeago = time_ago_with_tooltip(commit.authored_date, placement: 'bottom') + - commit_text = _('%{commit_author_link} authored %{commit_timeago}') % { commit_author_link: commit_author_link, commit_timeago: commit_timeago } + #{ commit_text.html_safe } - - if commit.description? - %pre.commit-row-description.js-toggle-content.append-bottom-8 - = preserve(markdown_field(commit, :description)) + - if commit.description? + %pre.commit-row-description.js-toggle-content.append-bottom-8 + = preserve(markdown_field(commit, :description)) - .commit-actions.flex-row.d-none.d-sm-flex - - if request.xhr? - = render partial: 'projects/commit/signature', object: commit.signature - - else - = render partial: 'projects/commit/ajax_signature', locals: { commit: commit } + .commit-actions.flex-row.d-none.d-sm-flex + - if request.xhr? + = render partial: 'projects/commit/signature', object: commit.signature + - else + = render partial: 'projects/commit/ajax_signature', locals: { commit: commit } - - if commit.status(ref) - = render_commit_status(commit, ref: ref) + - if commit.status(ref) + = render_commit_status(commit, ref: ref) - .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } } + .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } } - .commit-sha-group - .label.label-monospace - = commit.short_id - = clipboard_button(text: commit.id, title: _("Copy commit SHA to clipboard"), class: "btn btn-default", container: "body") - = link_to_browse_code(project, commit) + .commit-sha-group + .label.label-monospace + = commit.short_id + = clipboard_button(text: commit.id, title: _("Copy commit SHA to clipboard"), class: "btn btn-default", container: "body") + = link_to_browse_code(project, commit) diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml index 3dd2842be4f..ed7fefba56d 100644 --- a/app/views/shared/milestones/_milestone.html.haml +++ b/app/views/shared/milestones/_milestone.html.haml @@ -35,8 +35,8 @@ .col-sm-2 .milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end - if @project - - if can?(current_user, :admin_milestone, milestone.project) and milestone.active? - - if @project.group + - if can_admin_project_milestones? and milestone.active? + - if can_admin_group_milestones? %button.js-promote-project-milestone-button.btn.btn-blank.btn-sm.btn-grouped.has-tooltip{ title: _('Promote to Group Milestone'), disabled: true, type: 'button', diff --git a/changelogs/unreleased/51527-xss-in-mr-source-branch.yml b/changelogs/unreleased/51527-xss-in-mr-source-branch.yml new file mode 100644 index 00000000000..dae277b6413 --- /dev/null +++ b/changelogs/unreleased/51527-xss-in-mr-source-branch.yml @@ -0,0 +1,5 @@ +--- +title: Fix XSS in merge request source branch name +merge_request: +author: +type: security diff --git a/changelogs/unreleased/redact-links-dev.yml b/changelogs/unreleased/redact-links-dev.yml new file mode 100644 index 00000000000..338e7965465 --- /dev/null +++ b/changelogs/unreleased/redact-links-dev.yml @@ -0,0 +1,5 @@ +--- +title: Redact personal tokens in unsubscribe links. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-11-4-2717-fix-issue-title-xss.yml b/changelogs/unreleased/security-11-4-2717-fix-issue-title-xss.yml new file mode 100644 index 00000000000..12dfa48c6aa --- /dev/null +++ b/changelogs/unreleased/security-11-4-2717-fix-issue-title-xss.yml @@ -0,0 +1,5 @@ +--- +title: Escape entity title while autocomplete template rendering to prevent XSS +merge_request: 2571 +author: +type: security diff --git a/changelogs/unreleased/security-11-4-2717-xss-username-autocomplete.yml b/changelogs/unreleased/security-11-4-2717-xss-username-autocomplete.yml new file mode 100644 index 00000000000..d9b1015eeb4 --- /dev/null +++ b/changelogs/unreleased/security-11-4-2717-xss-username-autocomplete.yml @@ -0,0 +1,5 @@ +--- +title: Escape user fullname while rendering autocomplete template to prevent XSS +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-11-4-xss-in-markdown-following-unrecognized-html-element.yml b/changelogs/unreleased/security-11-4-xss-in-markdown-following-unrecognized-html-element.yml new file mode 100644 index 00000000000..16c4474aadd --- /dev/null +++ b/changelogs/unreleased/security-11-4-xss-in-markdown-following-unrecognized-html-element.yml @@ -0,0 +1,5 @@ +--- +title: Fix possible XSS attack in Markdown urls with spaces +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-182-update-workhorse.yml b/changelogs/unreleased/security-182-update-workhorse.yml new file mode 100644 index 00000000000..76850901b68 --- /dev/null +++ b/changelogs/unreleased/security-182-update-workhorse.yml @@ -0,0 +1,5 @@ +--- +title: Redact sensitive information on gitlab-workhorse log +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-2736-prometheus-ssrf.yml b/changelogs/unreleased/security-2736-prometheus-ssrf.yml new file mode 100644 index 00000000000..9d0dda8a75f --- /dev/null +++ b/changelogs/unreleased/security-2736-prometheus-ssrf.yml @@ -0,0 +1,5 @@ +--- +title: Do not follow redirects in Prometheus service when making http requests to the configured api url +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-51113-hash_personal_access_tokens.yml b/changelogs/unreleased/security-51113-hash_personal_access_tokens.yml new file mode 100644 index 00000000000..4cebe814148 --- /dev/null +++ b/changelogs/unreleased/security-51113-hash_personal_access_tokens.yml @@ -0,0 +1,5 @@ +--- +title: Persist only SHA digest of PersonalAccessToken#token +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-bvl-exposure-in-commits-list.yml b/changelogs/unreleased/security-bvl-exposure-in-commits-list.yml new file mode 100644 index 00000000000..0361fb0c041 --- /dev/null +++ b/changelogs/unreleased/security-bvl-exposure-in-commits-list.yml @@ -0,0 +1,5 @@ +--- +title: Don't expose confidential information in commit message list +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-email-change-notification.yml b/changelogs/unreleased/security-email-change-notification.yml new file mode 100644 index 00000000000..45075ff20bb --- /dev/null +++ b/changelogs/unreleased/security-email-change-notification.yml @@ -0,0 +1,5 @@ +--- +title: Provide email notification when a user changes their email address +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-fix-pat-web-access.yml b/changelogs/unreleased/security-fix-pat-web-access.yml new file mode 100644 index 00000000000..62ffb908fe5 --- /dev/null +++ b/changelogs/unreleased/security-fix-pat-web-access.yml @@ -0,0 +1,5 @@ +--- +title: Restrict Personal Access Tokens to API scope on web requests +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-guest-comments.yml b/changelogs/unreleased/security-guest-comments.yml new file mode 100644 index 00000000000..2c99512433b --- /dev/null +++ b/changelogs/unreleased/security-guest-comments.yml @@ -0,0 +1,5 @@ +--- +title: Fixed ability to comment on locked/confidential issues. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-guest-comments_2.yml b/changelogs/unreleased/security-guest-comments_2.yml new file mode 100644 index 00000000000..be6f2d6a490 --- /dev/null +++ b/changelogs/unreleased/security-guest-comments_2.yml @@ -0,0 +1,5 @@ +--- +title: Fixed ability of guest users to edit/delete comments on locked or confidential issues. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-issue_51301.yml b/changelogs/unreleased/security-issue_51301.yml new file mode 100644 index 00000000000..cf8ebb54b1c --- /dev/null +++ b/changelogs/unreleased/security-issue_51301.yml @@ -0,0 +1,5 @@ +--- +title: Fix milestone promotion authorization check +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-kubeclient-ssrf.yml b/changelogs/unreleased/security-kubeclient-ssrf.yml new file mode 100644 index 00000000000..45fc41029fc --- /dev/null +++ b/changelogs/unreleased/security-kubeclient-ssrf.yml @@ -0,0 +1,5 @@ +--- +title: Monkey kubeclient to not follow any redirects. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-mermaid-xss.yml b/changelogs/unreleased/security-mermaid-xss.yml new file mode 100644 index 00000000000..bcf93ef37ff --- /dev/null +++ b/changelogs/unreleased/security-mermaid-xss.yml @@ -0,0 +1,5 @@ +--- +title: Configure mermaid to not render HTML content in diagrams +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-pages-toctou-race.yml b/changelogs/unreleased/security-pages-toctou-race.yml new file mode 100644 index 00000000000..1c055f6087f --- /dev/null +++ b/changelogs/unreleased/security-pages-toctou-race.yml @@ -0,0 +1,6 @@ +--- +title: Fix a possible symlink time of check to time of use race condition in GitLab + Pages +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-private-group-11-5.yml b/changelogs/unreleased/security-private-group-11-5.yml new file mode 100644 index 00000000000..dbb7794dfed --- /dev/null +++ b/changelogs/unreleased/security-private-group-11-5.yml @@ -0,0 +1,6 @@ +--- +title: Removed ability to see private group names when the group id is entered in + the url. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-stored-xss-for-environments.yml b/changelogs/unreleased/security-stored-xss-for-environments.yml new file mode 100644 index 00000000000..5d78ca00942 --- /dev/null +++ b/changelogs/unreleased/security-stored-xss-for-environments.yml @@ -0,0 +1,5 @@ +--- +title: Fix stored XSS for Environments +merge_request: +author: +type: security diff --git a/changelogs/unreleased/sh-fix-hipchat-ssrf.yml b/changelogs/unreleased/sh-fix-hipchat-ssrf.yml new file mode 100644 index 00000000000..cdc95a34fcf --- /dev/null +++ b/changelogs/unreleased/sh-fix-hipchat-ssrf.yml @@ -0,0 +1,5 @@ +--- +title: Prevent SSRF attacks in HipChat integration +merge_request: +author: +type: security diff --git a/changelogs/unreleased/sh-fix-wiki-security-issue-53072.yml b/changelogs/unreleased/sh-fix-wiki-security-issue-53072.yml new file mode 100644 index 00000000000..ac6ab7cc3f4 --- /dev/null +++ b/changelogs/unreleased/sh-fix-wiki-security-issue-53072.yml @@ -0,0 +1,5 @@ +--- +title: Validate Wiki attachments are valid temporary files +merge_request: +author: +type: security diff --git a/config/application.rb b/config/application.rb index 9074cf02c46..78d6802e22b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -94,6 +94,8 @@ module Gitlab # - Webhook URLs (:hook) # - Sentry DSN (:sentry_dsn) # - File content from Web Editor (:content) + # + # NOTE: It is **IMPORTANT** to also update gitlab-workhorse's filter when adding parameters here! config.filter_parameters += [/token$/, /password/, /secret/, /key$/] config.filter_parameters += %i( certificate diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 179e00cdbd0..67eabb0b4fc 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -103,6 +103,9 @@ Devise.setup do |config| # Send a notification email when the user's password is changed config.send_password_change_notification = true + # Send a notification email when the user's email is changed + config.send_email_changed_notification = true + # ==> Configuration for :validatable # Range for password length. Default is 6..128. config.password_length = 8..128 diff --git a/config/initializers/rack_attack_global.rb b/config/initializers/rack_attack_global.rb index 45963831c41..86cb930eca9 100644 --- a/config/initializers/rack_attack_global.rb +++ b/config/initializers/rack_attack_global.rb @@ -33,22 +33,22 @@ class Rack::Attack throttle('throttle_authenticated_api', Gitlab::Throttle.authenticated_api_options) do |req| Gitlab::Throttle.settings.throttle_authenticated_api_enabled && req.api_request? && - req.authenticated_user_id + req.authenticated_user_id([:api]) end throttle('throttle_authenticated_web', Gitlab::Throttle.authenticated_web_options) do |req| Gitlab::Throttle.settings.throttle_authenticated_web_enabled && req.web_request? && - req.authenticated_user_id + req.authenticated_user_id([:api, :rss, :ics]) end class Request def unauthenticated? - !authenticated_user_id + !authenticated_user_id([:api, :rss, :ics]) end - def authenticated_user_id - Gitlab::Auth::RequestAuthenticator.new(self).user&.id + def authenticated_user_id(request_formats) + Gitlab::Auth::RequestAuthenticator.new(self).user(request_formats)&.id end def api_request? diff --git a/db/migrate/20181108091549_cleanup_environments_external_url.rb b/db/migrate/20181108091549_cleanup_environments_external_url.rb new file mode 100644 index 00000000000..8d6c20a4b15 --- /dev/null +++ b/db/migrate/20181108091549_cleanup_environments_external_url.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CleanupEnvironmentsExternalUrl < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + update_column_in_batches(:environments, :external_url, nil) do |table, query| + query.where(table[:external_url].matches('javascript://%')) + end + end + + def down + end +end diff --git a/db/schema.rb b/db/schema.rb index 50768d2693c..cab00a4dbf5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20181014121030) do +ActiveRecord::Schema.define(version: 20181108091549) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/doc/workflow/notifications.md b/doc/workflow/notifications.md index 731c9209224..0cd831b111a 100644 --- a/doc/workflow/notifications.md +++ b/doc/workflow/notifications.md @@ -63,6 +63,8 @@ Below is the table of events users can be notified of: |------------------------------|-------------------------------------------------------------------|------------------------------| | New SSH key added | User | Security email, always sent. | | New email added | User | Security email, always sent. | +| Email changed | User | Security email, always sent. | +| Password changed | User | Security email, always sent. | | New user created | User | Sent on user creation, except for omniauth (LDAP)| | User added to project | User | Sent when user is added to project | | Project access level changed | User | Sent when user project access level is changed | diff --git a/lib/banzai/filter/spaced_link_filter.rb b/lib/banzai/filter/spaced_link_filter.rb index a27f1d46863..c6a3a763c23 100644 --- a/lib/banzai/filter/spaced_link_filter.rb +++ b/lib/banzai/filter/spaced_link_filter.rb @@ -17,6 +17,9 @@ module Banzai # This is a small extension to the CommonMark spec. If they start allowing # spaces in urls, we could then remove this filter. # + # Note: Filter::SanitizationFilter should always be run sometime after this filter + # to prevent XSS attacks + # class SpacedLinkFilter < HTML::Pipeline::Filter include ActionView::Helpers::TagHelper diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index bd34614f149..227b6c8d0b5 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -10,13 +10,16 @@ module Banzai def self.filters @filters ||= FilterArray[ Filter::PlantumlFilter, + + # Must always be before the SanitizationFilter to prevent XSS attacks + Filter::SpacedLinkFilter, + Filter::SanitizationFilter, Filter::SyntaxHighlightFilter, Filter::MathFilter, Filter::ColorFilter, Filter::MermaidFilter, - Filter::SpacedLinkFilter, Filter::VideoLinkFilter, Filter::ImageLazyLoadFilter, Filter::ImageLinkFilter, diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb index 66de52506ce..7a9dbe02748 100644 --- a/lib/gitlab/auth/request_authenticator.rb +++ b/lib/gitlab/auth/request_authenticator.rb @@ -11,12 +11,18 @@ module Gitlab @request = request end - def user - find_sessionless_user || find_user_from_warden + def user(request_formats) + request_formats.each do |format| + user = find_sessionless_user(format) + + return user if user + end + + find_user_from_warden end - def find_sessionless_user - find_user_from_access_token || find_user_from_feed_token + def find_sessionless_user(request_format) + find_user_from_web_access_token(request_format) || find_user_from_feed_token(request_format) rescue Gitlab::Auth::AuthenticationError nil end diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb index a0b8d44c544..76f3a26eafa 100644 --- a/lib/gitlab/auth/user_auth_finders.rb +++ b/lib/gitlab/auth/user_auth_finders.rb @@ -25,8 +25,8 @@ module Gitlab current_request.env['warden']&.authenticate if verified_request? end - def find_user_from_feed_token - return unless rss_request? || ics_request? + def find_user_from_feed_token(request_format) + return unless valid_rss_format?(request_format) # NOTE: feed_token was renamed from rss_token but both needs to be supported because # users might have already added the feed to their RSS reader before the rename @@ -36,6 +36,17 @@ module Gitlab User.find_by_feed_token(token) || raise(UnauthorizedError) end + # We only allow Private Access Tokens with `api` scope to be used by web + # requests on RSS feeds or ICS files for backwards compatibility. + # It is also used by GraphQL/API requests. + def find_user_from_web_access_token(request_format) + return unless access_token && valid_web_access_format?(request_format) + + validate_access_token!(scopes: [:api]) + + access_token.user || raise(UnauthorizedError) + end + def find_user_from_access_token return unless access_token @@ -107,6 +118,26 @@ module Gitlab @current_request ||= ensure_action_dispatch_request(request) end + def valid_web_access_format?(request_format) + case request_format + when :rss + rss_request? + when :ics + ics_request? + when :api + api_request? + end + end + + def valid_rss_format?(request_format) + case request_format + when :rss + rss_request? + when :ics + ics_request? + end + end + def rss_request? current_request.path.ends_with?('.atom') || current_request.format.atom? end @@ -114,6 +145,10 @@ module Gitlab def ics_request? current_request.path.ends_with?('.ics') || current_request.format.ics? end + + def api_request? + current_request.path.starts_with?("/api/") + end end end end diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index a2cce9151c3..e48d2e39104 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -109,12 +109,14 @@ module Gitlab end def internal_web?(uri) - uri.hostname == config.gitlab.host && + uri.scheme == config.gitlab.protocol && + uri.hostname == config.gitlab.host && (uri.port.blank? || uri.port == config.gitlab.port) end def internal_shell?(uri) - uri.hostname == config.gitlab_shell.ssh_host && + uri.scheme == 'ssh' && + uri.hostname == config.gitlab_shell.ssh_host && (uri.port.blank? || uri.port == config.gitlab_shell.ssh_port) end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 04747c3c537..c848ab3fa4a 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -107,59 +107,6 @@ describe ApplicationController do end end - describe "#authenticate_user_from_personal_access_token!" do - before do - stub_authentication_activity_metrics(debug: false) - end - - controller(described_class) do - def index - render text: 'authenticated' - end - end - - let(:personal_access_token) { create(:personal_access_token, user: user) } - - context "when the 'personal_access_token' param is populated with the personal access token" do - it "logs the user in" do - expect(authentication_metrics) - .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) - .and increment(:user_sessionless_authentication_counter) - - get :index, private_token: personal_access_token.token - - expect(response).to have_gitlab_http_status(200) - expect(response.body).to eq('authenticated') - end - end - - context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do - it "logs the user in" do - expect(authentication_metrics) - .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) - .and increment(:user_sessionless_authentication_counter) - - @request.headers["PRIVATE-TOKEN"] = personal_access_token.token - get :index - - expect(response).to have_gitlab_http_status(200) - expect(response.body).to eq('authenticated') - end - end - - it "doesn't log the user in otherwise" do - expect(authentication_metrics) - .to increment(:user_unauthenticated_counter) - - get :index, private_token: "token" - - expect(response.status).not_to eq(200) - expect(response.body).not_to eq('authenticated') - end - end - describe 'session expiration' do controller(described_class) do # The anonymous controller will report 401 and fail to run any actions. @@ -248,74 +195,6 @@ describe ApplicationController do end end - describe '#authenticate_sessionless_user!' do - before do - stub_authentication_activity_metrics(debug: false) - end - - describe 'authenticating a user from a feed token' do - controller(described_class) do - def index - render text: 'authenticated' - end - end - - context "when the 'feed_token' param is populated with the feed token" do - context 'when the request format is atom' do - it "logs the user in" do - expect(authentication_metrics) - .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) - .and increment(:user_sessionless_authentication_counter) - - get :index, feed_token: user.feed_token, format: :atom - - expect(response).to have_gitlab_http_status 200 - expect(response.body).to eq 'authenticated' - end - end - - context 'when the request format is ics' do - it "logs the user in" do - expect(authentication_metrics) - .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) - .and increment(:user_sessionless_authentication_counter) - - get :index, feed_token: user.feed_token, format: :ics - - expect(response).to have_gitlab_http_status 200 - expect(response.body).to eq 'authenticated' - end - end - - context 'when the request format is neither atom nor ics' do - it "doesn't log the user in" do - expect(authentication_metrics) - .to increment(:user_unauthenticated_counter) - - get :index, feed_token: user.feed_token - - expect(response.status).not_to have_gitlab_http_status 200 - expect(response.body).not_to eq 'authenticated' - end - end - end - - context "when the 'feed_token' param is populated with an invalid feed token" do - it "doesn't log the user" do - expect(authentication_metrics) - .to increment(:user_unauthenticated_counter) - - get :index, feed_token: 'token', format: :atom - - expect(response.status).not_to eq 200 - expect(response.body).not_to eq 'authenticated' - end - end - end - end - describe '#route_not_found' do it 'renders 404 if authenticated' do allow(controller).to receive(:current_user).and_return(user) @@ -581,36 +460,6 @@ describe ApplicationController do expect(response).to have_gitlab_http_status(200) end - - context 'for sessionless users' do - render_views - - before do - sign_out user - end - - it 'renders a 403 when the sessionless user did not accept the terms' do - get :index, feed_token: user.feed_token, format: :atom - - expect(response).to have_gitlab_http_status(403) - end - - it 'renders the error message when the format was html' do - get :index, - private_token: create(:personal_access_token, user: user).token, - format: :html - - expect(response.body).to have_content /accept the terms of service/i - end - - it 'renders a 200 when the sessionless user accepted the terms' do - accept_terms(user) - - get :index, feed_token: user.feed_token, format: :atom - - expect(response).to have_gitlab_http_status(200) - end - end end end diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb new file mode 100644 index 00000000000..2975205e09c --- /dev/null +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe Dashboard::ProjectsController do + it_behaves_like 'authenticates sessionless user', :index, :atom +end diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb index b4a731fd3a3..e2c799f5205 100644 --- a/spec/controllers/dashboard/todos_controller_spec.rb +++ b/spec/controllers/dashboard/todos_controller_spec.rb @@ -42,6 +42,16 @@ describe Dashboard::TodosController do end end + context 'group authorization' do + it 'renders 404 when user does not have read access on given group' do + unauthorized_group = create(:group, :private) + + get :index, group_id: unauthorized_group.id + + expect(response).to have_gitlab_http_status(404) + end + end + context 'when using pagination' do let(:last_page) { user.todos.page.total_pages } let!(:issues) { create_list(:issue, 3, project: project, assignees: [user]) } diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 187542ba30c..c857a78d5e8 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -1,21 +1,26 @@ require 'spec_helper' describe DashboardController do - let(:user) { create(:user) } - let(:project) { create(:project) } + context 'signed in' do + let(:user) { create(:user) } + let(:project) { create(:project) } - before do - project.add_maintainer(user) - sign_in(user) - end + before do + project.add_maintainer(user) + sign_in(user) + end - describe 'GET issues' do - it_behaves_like 'issuables list meta-data', :issue, :issues - it_behaves_like 'issuables requiring filter', :issues - end + describe 'GET issues' do + it_behaves_like 'issuables list meta-data', :issue, :issues + it_behaves_like 'issuables requiring filter', :issues + end - describe 'GET merge requests' do - it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests - it_behaves_like 'issuables requiring filter', :merge_requests + describe 'GET merge requests' do + it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests + it_behaves_like 'issuables requiring filter', :merge_requests + end end + + it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first + it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index 1449036e148..949ad532365 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -52,15 +52,58 @@ describe GraphqlController do end end + context 'token authentication' do + before do + stub_authentication_activity_metrics(debug: false) + end + + let(:user) { create(:user, username: 'Simon') } + let(:personal_access_token) { create(:personal_access_token, user: user) } + + context "when the 'personal_access_token' param is populated with the personal access token" do + it 'logs the user in' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + + run_test_query!(private_token: personal_access_token.token) + + expect(response).to have_gitlab_http_status(200) + expect(query_response).to eq('echo' => '"Simon" says: test success') + end + end + + context 'when the personal access token has no api scope' do + it 'does not log the user in' do + personal_access_token.update(scopes: [:read_user]) + + run_test_query!(private_token: personal_access_token.token) + + expect(response).to have_gitlab_http_status(200) + + expect(query_response).to eq('echo' => 'nil says: test success') + end + end + + context 'without token' do + it 'shows public data' do + run_test_query! + + expect(query_response).to eq('echo' => 'nil says: test success') + end + end + end + # Chosen to exercise all the moving parts in GraphqlController#execute - def run_test_query!(variables: { 'text' => 'test success' }) + def run_test_query!(variables: { 'text' => 'test success' }, private_token: nil) query = <<~QUERY query Echo($text: String) { echo(text: $text) } QUERY - post :execute, query: query, operationName: 'Echo', variables: variables + post :execute, query: query, operationName: 'Echo', variables: variables, private_token: private_token end def query_response diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index a099cdafa58..2ba0f6ac518 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -606,4 +606,24 @@ describe GroupsController do end end end + + context 'token authentication' do + it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do + before do + default_params.merge!(id: group) + end + end + + it_behaves_like 'authenticates sessionless user', :issues, :atom, public: true do + before do + default_params.merge!(id: group, author_id: user.id) + end + end + + it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics, public: true do + before do + default_params.merge!(id: group) + end + end + end end diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb index a43bdd3ea80..5c72dab698c 100644 --- a/spec/controllers/projects/commits_controller_spec.rb +++ b/spec/controllers/projects/commits_controller_spec.rb @@ -5,87 +5,115 @@ describe Projects::CommitsController do let(:user) { create(:user) } before do - sign_in(user) project.add_maintainer(user) end - describe "GET commits_root" do - context "no ref is provided" do - it 'should redirect to the default branch of the project' do - get(:commits_root, - namespace_id: project.namespace, - project_id: project) + context 'signed in' do + before do + sign_in(user) + end + + describe "GET commits_root" do + context "no ref is provided" do + it 'should redirect to the default branch of the project' do + get(:commits_root, + namespace_id: project.namespace, + project_id: project) - expect(response).to redirect_to project_commits_path(project) + expect(response).to redirect_to project_commits_path(project) + end end end - end - describe "GET show" do - render_views + describe "GET show" do + render_views - context 'with file path' do - before do - get(:show, - namespace_id: project.namespace, - project_id: project, - id: id) - end + context 'with file path' do + before do + get(:show, + namespace_id: project.namespace, + project_id: project, + id: id) + end - context "valid branch, valid file" do - let(:id) { 'master/README.md' } + context "valid branch, valid file" do + let(:id) { 'master/README.md' } - it { is_expected.to respond_with(:success) } - end + it { is_expected.to respond_with(:success) } + end - context "valid branch, invalid file" do - let(:id) { 'master/invalid-path.rb' } + context "valid branch, invalid file" do + let(:id) { 'master/invalid-path.rb' } - it { is_expected.to respond_with(:not_found) } - end + it { is_expected.to respond_with(:not_found) } + end - context "invalid branch, valid file" do - let(:id) { 'invalid-branch/README.md' } + context "invalid branch, valid file" do + let(:id) { 'invalid-branch/README.md' } - it { is_expected.to respond_with(:not_found) } + it { is_expected.to respond_with(:not_found) } + end end - end - context "when the ref name ends in .atom" do - context "when the ref does not exist with the suffix" do - before do - get(:show, - namespace_id: project.namespace, - project_id: project, - id: "master.atom") + context "when the ref name ends in .atom" do + context "when the ref does not exist with the suffix" do + before do + get(:show, + namespace_id: project.namespace, + project_id: project, + id: "master.atom") + end + + it "renders as atom" do + expect(response).to be_success + expect(response.content_type).to eq('application/atom+xml') + end + + it 'renders summary with type=html' do + expect(response.body).to include('<summary type="html">') + end end - it "renders as atom" do - expect(response).to be_success - expect(response.content_type).to eq('application/atom+xml') - end + context "when the ref exists with the suffix" do + before do + commit = project.repository.commit('master') - it 'renders summary with type=html' do - expect(response.body).to include('<summary type="html">') + allow_any_instance_of(Repository).to receive(:commit).and_call_original + allow_any_instance_of(Repository).to receive(:commit).with('master.atom').and_return(commit) + + get(:show, + namespace_id: project.namespace, + project_id: project, + id: "master.atom") + end + + it "renders as HTML" do + expect(response).to be_success + expect(response.content_type).to eq('text/html') + end end end + end + end - context "when the ref exists with the suffix" do + context 'token authentication' do + context 'public project' do + it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do before do - commit = project.repository.commit('master') + public_project = create(:project, :repository, :public) - allow_any_instance_of(Repository).to receive(:commit).and_call_original - allow_any_instance_of(Repository).to receive(:commit).with('master.atom').and_return(commit) - - get(:show, - namespace_id: project.namespace, - project_id: project, - id: "master.atom") + default_params.merge!(namespace_id: public_project.namespace, project_id: public_project, id: "master.atom") end + end + end + + context 'private project' do + it_behaves_like 'authenticates sessionless user', :show, :atom, public: false do + before do + private_project = create(:project, :repository, :private) + private_project.add_maintainer(user) - it "renders as HTML" do - expect(response).to be_success - expect(response.content_type).to eq('text/html') + default_params.merge!(namespace_id: private_project.namespace, project_id: private_project, id: "master.atom") end end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 9df77560320..240445f7552 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1061,4 +1061,40 @@ describe Projects::IssuesController do end end end + + context 'private project with token authentication' do + let(:private_project) { create(:project, :private) } + + it_behaves_like 'authenticates sessionless user', :index, :atom do + before do + default_params.merge!(project_id: private_project, namespace_id: private_project.namespace) + + private_project.add_maintainer(user) + end + end + + it_behaves_like 'authenticates sessionless user', :calendar, :ics do + before do + default_params.merge!(project_id: private_project, namespace_id: private_project.namespace) + + private_project.add_maintainer(user) + end + end + end + + context 'public project with token authentication' do + let(:public_project) { create(:project, :public) } + + it_behaves_like 'authenticates sessionless user', :index, :atom, public: true do + before do + default_params.merge!(project_id: public_project, namespace_id: public_project.namespace) + end + end + + it_behaves_like 'authenticates sessionless user', :calendar, :ics, public: true do + before do + default_params.merge!(project_id: public_project, namespace_id: public_project.namespace) + end + end + end end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 3190f1ce9d4..500df0ca9b6 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -143,11 +143,27 @@ describe Projects::MilestonesController do end describe '#promote' do + let(:group) { create(:group) } + + before do + project.update(namespace: group) + end + + context 'when user does not have permission to promote milestone' do + before do + group.add_guest(user) + end + + it 'renders 404' do + post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid + + expect(response).to have_gitlab_http_status(404) + end + end + context 'promotion succeeds' do before do - group = create(:group) group.add_developer(user) - milestone.project.update(namespace: group) end it 'shows group milestone' do @@ -166,12 +182,17 @@ describe Projects::MilestonesController do end end - context 'promotion fails' do - it 'shows project milestone' do + context 'when user cannot admin group milestones' do + before do + project.add_developer(user) + end + + it 'renders 404' do + project.update(namespace: user.namespace) + post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid - expect(response).to redirect_to(project_milestone_path(project, milestone)) - expect(flash[:alert]).to eq('Promotion failed - Project does not belong to a group.') + expect(response).to have_gitlab_http_status(404) end end end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index e48c9dea976..37f5834b4da 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -252,14 +252,14 @@ describe Projects::NotesController do def post_create(extra_params = {}) post :create, { - note: { note: 'some other note' }, - namespace_id: project.namespace, - project_id: project, - target_type: 'merge_request', - target_id: merge_request.id, - note_project_id: forked_project.id, - in_reply_to_discussion_id: existing_comment.discussion_id - }.merge(extra_params) + note: { note: 'some other note', noteable_id: merge_request.id }, + namespace_id: project.namespace, + project_id: project, + target_type: 'merge_request', + target_id: merge_request.id, + note_project_id: forked_project.id, + in_reply_to_discussion_id: existing_comment.discussion_id + }.merge(extra_params) end context 'when the note_project_id is not correct' do @@ -293,6 +293,30 @@ describe Projects::NotesController do end end + context 'when target_id and noteable_id do not match' do + let(:locked_issue) { create(:issue, :locked, project: project) } + let(:issue) {create(:issue, project: project)} + + before do + project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) + project.project_member(user).destroy + end + + it 'uses target_id and ignores noteable_id' do + request_params = { + note: { note: 'some note', noteable_type: 'Issue', noteable_id: locked_issue.id }, + target_type: 'issue', + target_id: issue.id, + project_id: project, + namespace_id: project.namespace + } + + expect { post :create, request_params }.to change { issue.notes.count }.by(1) + .and change { locked_issue.notes.count }.by(0) + expect(response).to have_gitlab_http_status(302) + end + end + context 'when the merge request discussion is locked' do before do project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) @@ -345,35 +369,60 @@ describe Projects::NotesController do end describe 'PUT update' do - let(:request_params) do - { - namespace_id: project.namespace, - project_id: project, - id: note, - format: :json, - note: { - note: "New comment" + context "should update the note with a valid issue" do + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + id: note, + format: :json, + note: { + note: "New comment" + } } - } - end + end - before do - sign_in(note.author) - project.add_developer(note.author) + before do + sign_in(note.author) + project.add_developer(note.author) + end + + it "updates the note" do + expect { put :update, request_params }.to change { note.reload.note } + end end + context "doesnt update the note" do + let(:issue) { create(:issue, :confidential, project: project) } + let(:note) { create(:note, noteable: issue, project: project) } - it "updates the note" do - expect { put :update, request_params }.to change { note.reload.note } + before do + sign_in(user) + project.add_guest(user) + end + + it "disallows edits when the issue is confidential and the user has guest permissions" do + request_params = { + namespace_id: project.namespace, + project_id: project, + id: note, + format: :json, + note: { + note: "New comment" + } + } + expect { put :update, request_params }.not_to change { note.reload.note } + expect(response).to have_gitlab_http_status(404) + end end end describe 'DELETE destroy' do let(:request_params) do { - namespace_id: project.namespace, - project_id: project, - id: note, - format: :js + namespace_id: project.namespace, + project_id: project, + id: note, + format: :js } end diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb index c48f41ca12e..6fbf75d0259 100644 --- a/spec/controllers/projects/tags_controller_spec.rb +++ b/spec/controllers/projects/tags_controller_spec.rb @@ -35,4 +35,26 @@ describe Projects::TagsController do it { is_expected.to respond_with(:not_found) } end end + + context 'private project with token authentication' do + let(:private_project) { create(:project, :repository, :private) } + + it_behaves_like 'authenticates sessionless user', :index, :atom do + before do + default_params.merge!(project_id: private_project, namespace_id: private_project.namespace) + + private_project.add_maintainer(user) + end + end + end + + context 'public project with token authentication' do + let(:public_project) { create(:project, :repository, :public) } + + it_behaves_like 'authenticates sessionless user', :index, :atom, public: true do + before do + default_params.merge!(project_id: public_project, namespace_id: public_project.namespace) + end + end + end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 3bc9cbe64c5..7849bec4762 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -882,6 +882,28 @@ describe ProjectsController do end end + context 'private project with token authentication' do + let(:private_project) { create(:project, :private) } + + it_behaves_like 'authenticates sessionless user', :show, :atom do + before do + default_params.merge!(id: private_project, namespace_id: private_project.namespace) + + private_project.add_maintainer(user) + end + end + end + + context 'public project with token authentication' do + let(:public_project) { create(:project, :public) } + + it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do + before do + default_params.merge!(id: public_project, namespace_id: public_project.namespace) + end + end + end + def project_moved_message(redirect_route, project) "Project '#{redirect_route.path}' was moved to '#{project.full_path}'. Please update any links and bookmarks that may still have the old path." end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 071f96a729e..fe438e71e9e 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -395,6 +395,14 @@ describe UsersController do end end + context 'token authentication' do + it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do + before do + default_params.merge!(username: user.username) + end + end + end + def user_moved_message(redirect_route, user) "User '#{redirect_route.path}' was moved to '#{user.full_path}'. Please update any links and bookmarks that may still have the old path." end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 85ba7d4097d..d9203b70e96 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -278,7 +278,7 @@ FactoryBot.define do trait :with_runner_session do after(:build) do |build| - build.build_runner_session(url: 'ws://localhost') + build.build_runner_session(url: 'https://localhost') end end end diff --git a/spec/features/issues/user_comments_on_issue_spec.rb b/spec/features/issues/user_comments_on_issue_spec.rb index ba5b80ed04b..b4b9a589ba3 100644 --- a/spec/features/issues/user_comments_on_issue_spec.rb +++ b/spec/features/issues/user_comments_on_issue_spec.rb @@ -40,6 +40,18 @@ describe "User comments on issue", :js do expect(page.find('pre code').text).to eq code_block_content end + + it "does not render html content in mermaid" do + html_content = "<img onerror=location=`javascript\\u003aalert\\u0028document.domain\\u0029` src=x>" + mermaid_content = "graph LR\n B-->D(#{html_content});" + comment = "```mermaid\n#{mermaid_content}\n```" + + add_note(comment) + + wait_for_requests + + expect(page.find('svg.mermaid')).to have_content html_content + end end context "when editing comments" do diff --git a/spec/features/markdown/mermaid_spec.rb b/spec/features/markdown/mermaid_spec.rb index a25d701ee35..7008b361394 100644 --- a/spec/features/markdown/mermaid_spec.rb +++ b/spec/features/markdown/mermaid_spec.rb @@ -18,7 +18,7 @@ describe 'Mermaid rendering', :js do visit project_issue_path(project, issue) %w[A B C D].each do |label| - expect(page).to have_selector('svg foreignObject', text: label) + expect(page).to have_selector('svg text', text: label) end end end diff --git a/spec/features/milestones/user_promotes_milestone_spec.rb b/spec/features/milestones/user_promotes_milestone_spec.rb new file mode 100644 index 00000000000..df1bc502134 --- /dev/null +++ b/spec/features/milestones/user_promotes_milestone_spec.rb @@ -0,0 +1,32 @@ +require 'rails_helper' + +describe 'User promotes milestone' do + set(:group) { create(:group) } + set(:user) { create(:user) } + set(:project) { create(:project, namespace: group) } + set(:milestone) { create(:milestone, project: project) } + + context 'when user can admin group milestones' do + before do + group.add_developer(user) + sign_in(user) + visit(project_milestones_path(project)) + end + + it "shows milestone promote button" do + expect(page).to have_selector('.js-promote-project-milestone-button') + end + end + + context 'when user cannot admin group milestones' do + before do + project.add_developer(user) + sign_in(user) + visit(project_milestones_path(project)) + end + + it "does not show milestone promote button" do + expect(page).not_to have_selector('.js-promote-project-milestone-button') + end + end +end diff --git a/spec/features/projects/commits/user_browses_commits_spec.rb b/spec/features/projects/commits/user_browses_commits_spec.rb index 534cfe1eb12..2159adf49fc 100644 --- a/spec/features/projects/commits/user_browses_commits_spec.rb +++ b/spec/features/projects/commits/user_browses_commits_spec.rb @@ -4,10 +4,9 @@ describe 'User browses commits' do include RepoHelpers let(:user) { create(:user) } - let(:project) { create(:project, :repository, namespace: user.namespace) } + let(:project) { create(:project, :public, :repository, namespace: user.namespace) } before do - project.add_maintainer(user) sign_in(user) end @@ -127,6 +126,26 @@ describe 'User browses commits' do .and have_selector('entry summary', text: commit.description[0..10].delete("\r\n")) end + context 'when a commit links to a confidential issue' do + let(:confidential_issue) { create(:issue, confidential: true, title: 'Secret issue!', project: project) } + + before do + project.repository.create_file(user, 'dummy-file', 'dummy content', + branch_name: 'feature', + message: "Linking #{confidential_issue.to_reference}") + end + + context 'when the user cannot see confidential issues but was cached with a link', :use_clean_rails_memory_store_fragment_caching do + it 'does not render the confidential issue' do + visit project_commits_path(project, 'feature') + sign_in(create(:user)) + visit project_commits_path(project, 'feature') + + expect(page).not_to have_link(href: project_issue_path(project, confidential_issue)) + end + end + end + context 'master branch' do before do visit_commits_page diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb index df24cef0b8b..91b0499375d 100644 --- a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb @@ -104,5 +104,17 @@ describe Banzai::Pipeline::GfmPipeline do expect(output).to include("src=\"test%20image.png\"") end + + it 'sanitizes the fixed link' do + markdown_xss = "[xss](javascript: alert%28document.domain%29)" + output = described_class.to_html(markdown_xss, project: project) + + expect(output).not_to include("javascript") + + markdown_xss = "<invalidtag>\n[xss](javascript:alert%28document.domain%29)" + output = described_class.to_html(markdown_xss, project: project) + + expect(output).not_to include("javascript") + end end end diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb index 242ab4a91dd..3d979132880 100644 --- a/spec/lib/gitlab/auth/request_authenticator_spec.rb +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -19,17 +19,17 @@ describe Gitlab::Auth::RequestAuthenticator do allow_any_instance_of(described_class).to receive(:find_sessionless_user).and_return(sessionless_user) allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user) - expect(subject.user).to eq sessionless_user + expect(subject.user([:api])).to eq sessionless_user end it 'returns session user if no sessionless user found' do allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user) - expect(subject.user).to eq session_user + expect(subject.user([:api])).to eq session_user end it 'returns nil if no user found' do - expect(subject.user).to be_blank + expect(subject.user([:api])).to be_blank end it 'bubbles up exceptions' do @@ -42,26 +42,26 @@ describe Gitlab::Auth::RequestAuthenticator do let!(:feed_token_user) { build(:user) } it 'returns access_token user first' do - allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_return(access_token_user) + allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_return(access_token_user) allow_any_instance_of(described_class).to receive(:find_user_from_feed_token).and_return(feed_token_user) - expect(subject.find_sessionless_user).to eq access_token_user + expect(subject.find_sessionless_user([:api])).to eq access_token_user end it 'returns feed_token user if no access_token user found' do allow_any_instance_of(described_class).to receive(:find_user_from_feed_token).and_return(feed_token_user) - expect(subject.find_sessionless_user).to eq feed_token_user + expect(subject.find_sessionless_user([:api])).to eq feed_token_user end it 'returns nil if no user found' do - expect(subject.find_sessionless_user).to be_blank + expect(subject.find_sessionless_user([:api])).to be_blank end it 'rescue Gitlab::Auth::AuthenticationError exceptions' do - allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_raise(Gitlab::Auth::UnauthorizedError) + allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_raise(Gitlab::Auth::UnauthorizedError) - expect(subject.find_sessionless_user).to be_blank + expect(subject.find_sessionless_user([:api])).to be_blank end end end diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb index 454ad1589b9..5d3fbba264f 100644 --- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Auth::UserAuthFinders do 'rack.input' => '' } end - let(:request) { Rack::Request.new(env)} + let(:request) { Rack::Request.new(env) } def set_param(key, value) request.update_param(key, value) @@ -49,6 +49,7 @@ describe Gitlab::Auth::UserAuthFinders do describe '#find_user_from_feed_token' do context 'when the request format is atom' do before do + env['SCRIPT_NAME'] = 'url.atom' env['HTTP_ACCEPT'] = 'application/atom+xml' end @@ -56,17 +57,17 @@ describe Gitlab::Auth::UserAuthFinders do it 'returns user if valid feed_token' do set_param(:feed_token, user.feed_token) - expect(find_user_from_feed_token).to eq user + expect(find_user_from_feed_token(:rss)).to eq user end it 'returns nil if feed_token is blank' do - expect(find_user_from_feed_token).to be_nil + expect(find_user_from_feed_token(:rss)).to be_nil end it 'returns exception if invalid feed_token' do set_param(:feed_token, 'invalid_token') - expect { find_user_from_feed_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError) end end @@ -74,34 +75,38 @@ describe Gitlab::Auth::UserAuthFinders do it 'returns user if valid rssd_token' do set_param(:rss_token, user.feed_token) - expect(find_user_from_feed_token).to eq user + expect(find_user_from_feed_token(:rss)).to eq user end it 'returns nil if rss_token is blank' do - expect(find_user_from_feed_token).to be_nil + expect(find_user_from_feed_token(:rss)).to be_nil end it 'returns exception if invalid rss_token' do set_param(:rss_token, 'invalid_token') - expect { find_user_from_feed_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError) end end end context 'when the request format is not atom' do it 'returns nil' do + env['SCRIPT_NAME'] = 'json' + set_param(:feed_token, user.feed_token) - expect(find_user_from_feed_token).to be_nil + expect(find_user_from_feed_token(:rss)).to be_nil end end context 'when the request format is empty' do it 'the method call does not modify the original value' do + env['SCRIPT_NAME'] = 'url.atom' + env.delete('action_dispatch.request.formats') - find_user_from_feed_token + find_user_from_feed_token(:rss) expect(env['action_dispatch.request.formats']).to be_nil end @@ -111,8 +116,12 @@ describe Gitlab::Auth::UserAuthFinders do describe '#find_user_from_access_token' do let(:personal_access_token) { create(:personal_access_token, user: user) } + before do + env['SCRIPT_NAME'] = 'url.atom' + end + it 'returns nil if no access_token present' do - expect(find_personal_access_token).to be_nil + expect(find_user_from_access_token).to be_nil end context 'when validate_access_token! returns valid' do @@ -131,9 +140,59 @@ describe Gitlab::Auth::UserAuthFinders do end end + describe '#find_user_from_web_access_token' do + let(:personal_access_token) { create(:personal_access_token, user: user) } + + before do + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token + end + + it 'returns exception if token has no user' do + allow_any_instance_of(PersonalAccessToken).to receive(:user).and_return(nil) + + expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + + context 'no feed or API requests' do + it 'returns nil if the request is not RSS' do + expect(find_user_from_web_access_token(:rss)).to be_nil + end + + it 'returns nil if the request is not ICS' do + expect(find_user_from_web_access_token(:ics)).to be_nil + end + + it 'returns nil if the request is not API' do + expect(find_user_from_web_access_token(:api)).to be_nil + end + end + + it 'returns the user for RSS requests' do + env['SCRIPT_NAME'] = 'url.atom' + + expect(find_user_from_web_access_token(:rss)).to eq(user) + end + + it 'returns the user for ICS requests' do + env['SCRIPT_NAME'] = 'url.ics' + + expect(find_user_from_web_access_token(:ics)).to eq(user) + end + + it 'returns the user for API requests' do + env['SCRIPT_NAME'] = '/api/endpoint' + + expect(find_user_from_web_access_token(:api)).to eq(user) + end + end + describe '#find_personal_access_token' do let(:personal_access_token) { create(:personal_access_token, user: user) } + before do + env['SCRIPT_NAME'] = 'url.atom' + end + context 'passed as header' do it 'returns token if valid personal_access_token' do env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 8df0facdab3..35b550283b5 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -10,8 +10,8 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?(import_url)).to be false end - it 'allows imports from configured SSH host and port' do - import_url = "http://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git" + it 'allows mirroring from configured SSH host and port' do + import_url = "ssh://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git" expect(described_class.blocked_url?(import_url)).to be false end @@ -29,6 +29,14 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['http'])).to be true end + it 'returns true for bad protocol on configured web/SSH host and ports' do + web_url = "javascript://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git%0aalert(1)" + expect(described_class.blocked_url?(web_url)).to be true + + ssh_url = "javascript://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git%0aalert(1)" + expect(described_class.blocked_url?(ssh_url)).to be true + end + it 'returns true for localhost IPs' do expect(described_class.blocked_url?('https://0.0.0.0/foo/foo.git')).to be true expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index ff1a5aa2536..150c00e4bfe 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -522,7 +522,7 @@ describe Notify do let(:project_snippet) { create(:project_snippet, project: project) } let(:project_snippet_note) { create(:note_on_project_snippet, project: project, noteable: project_snippet) } - subject { described_class.note_snippet_email(project_snippet_note.author_id, project_snippet_note.id) } + subject { described_class.note_project_snippet_email(project_snippet_note.author_id, project_snippet_note.id) } it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { project_snippet } diff --git a/spec/migrations/cleanup_environments_external_url_spec.rb b/spec/migrations/cleanup_environments_external_url_spec.rb new file mode 100644 index 00000000000..07ddaf3d38f --- /dev/null +++ b/spec/migrations/cleanup_environments_external_url_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20181108091549_cleanup_environments_external_url.rb') + +describe CleanupEnvironmentsExternalUrl, :migration do + let(:environments) { table(:environments) } + let(:invalid_entries) { environments.where(environments.arel_table[:external_url].matches('javascript://%')) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + + before do + namespace = namespaces.create(name: 'foo', path: 'foo') + project = projects.create!(namespace_id: namespace.id) + + environments.create!(id: 1, project_id: project.id, name: 'poisoned', slug: 'poisoned', external_url: 'javascript://alert("1")') + end + + it 'clears every environment with a javascript external_url' do + expect do + subject.up + end.to change { invalid_entries.count }.from(1).to(0) + end + + it 'do not removes environments' do + expect do + subject.up + end.not_to change { environments.count } + end +end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 1783dd3206b..66f9f8e4ba6 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -517,7 +517,7 @@ describe Note do describe '#to_ability_name' do it 'returns snippet for a project snippet note' do - expect(build(:note_on_project_snippet).to_ability_name).to eq('snippet') + expect(build(:note_on_project_snippet).to_ability_name).to eq('project_snippet') end it 'returns personal_snippet for a personal snippet note' do diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 7afb1b4a8e3..ac92da6e1b1 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -11,6 +11,23 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do it { is_expected.to belong_to :project } end + context 'redirects' do + it 'does not follow redirects' do + redirect_to = 'https://redirected.example.com' + redirect_req_stub = stub_prometheus_request(prometheus_query_url('1'), status: 302, headers: { location: redirect_to }) + redirected_req_stub = stub_prometheus_request(redirect_to, body: { 'status': 'success' }) + + result = service.test + + # result = { success: false, result: error } + expect(result[:success]).to be_falsy + expect(result[:result]).to be_instance_of(Gitlab::PrometheusClient::Error) + + expect(redirect_req_stub).to have_been_requested + expect(redirected_req_stub).not_to have_been_requested + end + end + describe 'Validations' do context 'when manual_configuration is enabled' do before do diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb index e8096358f7d..7e25c53e77c 100644 --- a/spec/policies/note_policy_spec.rb +++ b/spec/policies/note_policy_spec.rb @@ -10,11 +10,50 @@ describe NotePolicy, mdoels: true do return @policies if @policies noteable ||= issue - note = create(:note, noteable: noteable, author: user, project: project) + 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 + 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) + expect(policy).to be_disallowed(:resolve_note) + expect(policy).to be_disallowed(:read_note) + end + end + + context 'when the note author can still see the noteable' do + before do + project.add_developer(user) + end + + it 'can edit the note' do + expect(policy).to be_allowed(:admin_note) + expect(policy).to be_allowed(:resolve_note) + expect(policy).to be_allowed(:read_note) + end + end + end + + context 'when the project is private' do + let(:project) { create(:project, :private, :repository) } + + context 'when the noteable is a commit' do + it_behaves_like 'a discussion with a private noteable' do + let(:noteable) { project.repository.head_commit } + end + end + end + context 'when the project is public' do context 'when the note author is not a project member' do it 'can edit a note' do @@ -24,14 +63,48 @@ describe NotePolicy, mdoels: true do end end - context 'when the noteable is a snippet' do + context 'when the noteable is a project snippet' do + it 'can edit note' do + policies = policies(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) + 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 + end + end + + context 'when the noteable is a personal snippet' do it 'can edit note' do - policies = policies(create(:project_snippet, project: project)) + policies = policies(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) end + + context 'when it is private' do + it 'can not edit nor read the note' do + policies = policies(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) + end + end + end + + context 'when a discussion is confidential' do + before do + issue.update_attribute(:confidential, true) + end + + it_behaves_like 'a discussion with a private noteable' end context 'when a discussion is locked' do diff --git a/spec/support/controllers/sessionless_auth_controller_shared_examples.rb b/spec/support/controllers/sessionless_auth_controller_shared_examples.rb new file mode 100644 index 00000000000..7e4958f177a --- /dev/null +++ b/spec/support/controllers/sessionless_auth_controller_shared_examples.rb @@ -0,0 +1,92 @@ +shared_examples 'authenticates sessionless user' do |path, format, params| + params ||= {} + + before do + stub_authentication_activity_metrics(debug: false) + end + + let(:user) { create(:user) } + let(:personal_access_token) { create(:personal_access_token, user: user) } + let(:default_params) { { format: format }.merge(params.except(:public) || {}) } + + context "when the 'personal_access_token' param is populated with the personal access token" do + it 'logs the user in' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + + get path, default_params.merge(private_token: personal_access_token.token) + + expect(response).to have_gitlab_http_status(200) + expect(controller.current_user).to eq(user) + end + + it 'does not log the user in if page is public', if: params[:public] do + get path, default_params + + expect(response).to have_gitlab_http_status(200) + expect(controller.current_user).to be_nil + end + end + + context 'when the personal access token has no api scope', unless: params[:public] do + it 'does not log the user in' do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + + personal_access_token.update(scopes: [:read_user]) + + get path, default_params.merge(private_token: personal_access_token.token) + + expect(response).not_to have_gitlab_http_status(200) + end + end + + context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do + it 'logs the user in' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + + @request.headers['PRIVATE-TOKEN'] = personal_access_token.token + get path, default_params + + expect(response).to have_gitlab_http_status(200) + end + end + + context "when the 'feed_token' param is populated with the feed token", if: format == :rss do + it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + + get path, default_params.merge(feed_token: user.feed_token) + + expect(response).to have_gitlab_http_status 200 + end + end + + context "when the 'feed_token' param is populated with an invalid feed token", if: format == :rss, unless: params[:public] do + it "logs the user" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + + get path, default_params.merge(feed_token: 'token') + + expect(response.status).not_to eq 200 + end + end + + it "doesn't log the user in otherwise", unless: params[:public] do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + + get path, default_params.merge(private_token: 'token') + + expect(response.status).not_to eq(200) + end +end diff --git a/spec/support/helpers/prometheus_helpers.rb b/spec/support/helpers/prometheus_helpers.rb index 4212be2cc88..ce1f9fce10d 100644 --- a/spec/support/helpers/prometheus_helpers.rb +++ b/spec/support/helpers/prometheus_helpers.rb @@ -49,11 +49,11 @@ module PrometheusHelpers "https://prometheus.example.com/api/v1/series?#{query}" end - def stub_prometheus_request(url, body: {}, status: 200) + def stub_prometheus_request(url, body: {}, status: 200, headers: {}) WebMock.stub_request(:get, url) .to_return({ status: status, - headers: { 'Content-Type' => 'application/json' }, + headers: { 'Content-Type' => 'application/json' }.merge(headers), body: body.to_json }) end |