diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-28 13:33:25 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-28 13:33:25 +0000 |
commit | 530b52cd42fb3b560f119e3d925db4597c0e0d9b (patch) | |
tree | e2079a04b47b652eb817f999f19efc11fbe2cc57 | |
parent | c70fc855befaae4fb88bec9016421ad98db5b182 (diff) | |
download | gitlab-ce-530b52cd42fb3b560f119e3d925db4597c0e0d9b.tar.gz |
Add latest changes from gitlab-org/security/gitlab@12-6-stable-ee
35 files changed, 544 insertions, 51 deletions
diff --git a/app/assets/javascripts/frequent_items/components/app.vue b/app/assets/javascripts/frequent_items/components/app.vue index 8cf939254c1..2ffecce0a56 100644 --- a/app/assets/javascripts/frequent_items/components/app.vue +++ b/app/assets/javascripts/frequent_items/components/app.vue @@ -5,7 +5,7 @@ import AccessorUtilities from '~/lib/utils/accessor'; import eventHub from '../event_hub'; import store from '../store/'; import { FREQUENT_ITEMS, STORAGE_KEY } from '../constants'; -import { isMobile, updateExistingFrequentItem } from '../utils'; +import { isMobile, updateExistingFrequentItem, sanitizeItem } from '../utils'; import FrequentItemsSearchInput from './frequent_items_search_input.vue'; import FrequentItemsList from './frequent_items_list.vue'; import frequentItemsMixin from './frequent_items_mixin'; @@ -64,7 +64,9 @@ export default { this.fetchFrequentItems(); } }, - logItemAccess(storageKey, item) { + logItemAccess(storageKey, unsanitizedItem) { + const item = sanitizeItem(unsanitizedItem); + if (!AccessorUtilities.isLocalStorageAccessSafe()) { return false; } diff --git a/app/assets/javascripts/frequent_items/components/frequent_items_list.vue b/app/assets/javascripts/frequent_items/components/frequent_items_list.vue index 67ffa97a046..0ece64692ae 100644 --- a/app/assets/javascripts/frequent_items/components/frequent_items_list.vue +++ b/app/assets/javascripts/frequent_items/components/frequent_items_list.vue @@ -1,6 +1,7 @@ <script> import FrequentItemsListItem from './frequent_items_list_item.vue'; import frequentItemsMixin from './frequent_items_mixin'; +import { sanitizeItem } from '../utils'; export default { components: { @@ -48,6 +49,9 @@ export default { ? this.translations.itemListErrorMessage : this.translations.itemListEmptyMessage; }, + sanitizedItems() { + return this.items.map(sanitizeItem); + }, }, }; </script> @@ -59,7 +63,7 @@ export default { {{ listEmptyMessage }} </li> <frequent-items-list-item - v-for="item in items" + v-for="item in sanitizedItems" v-else :key="item.id" :item-id="item.id" diff --git a/app/assets/javascripts/frequent_items/utils.js b/app/assets/javascripts/frequent_items/utils.js index aba692e4b99..09d88d34223 100644 --- a/app/assets/javascripts/frequent_items/utils.js +++ b/app/assets/javascripts/frequent_items/utils.js @@ -1,5 +1,6 @@ import _ from 'underscore'; import bp from '~/breakpoints'; +import sanitize from 'sanitize-html'; import { FREQUENT_ITEMS, HOUR_IN_MS } from './constants'; export const isMobile = () => { @@ -47,3 +48,9 @@ export const updateExistingFrequentItem = (frequentItem, item) => { lastAccessedOn: accessedOverHourAgo ? Date.now() : frequentItem.lastAccessedOn, }; }; + +export const sanitizeItem = item => ({ + ...item, + name: sanitize(item.name.toString(), { allowedTags: [] }), + namespace: sanitize(item.namespace.toString(), { allowedTags: [] }), +}); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f5306801c04..b343967ce71 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -5,6 +5,7 @@ require 'fogbugz' class ApplicationController < ActionController::Base include Gitlab::GonHelper + include Gitlab::NoCacheHeaders include GitlabRoutingHelper include PageLayoutHelper include SafeParamsHelper @@ -54,7 +55,6 @@ class ApplicationController < ActionController::Base # Adds `no-store` to the DEFAULT_CACHE_CONTROL, to prevent security # concerns due to caching private data. DEFAULT_GITLAB_CACHE_CONTROL = "#{ActionDispatch::Http::Cache::Response::DEFAULT_CACHE_CONTROL}, no-store" - DEFAULT_GITLAB_CONTROL_NO_CACHE = "#{DEFAULT_GITLAB_CACHE_CONTROL}, no-cache" rescue_from Encoding::CompatibilityError do |exception| log_exception(exception) @@ -246,9 +246,9 @@ class ApplicationController < ActionController::Base end def no_cache_headers - headers['Cache-Control'] = DEFAULT_GITLAB_CONTROL_NO_CACHE - headers['Pragma'] = 'no-cache' # HTTP 1.0 compatibility - headers['Expires'] = 'Fri, 01 Jan 1990 00:00:00 GMT' + DEFAULT_GITLAB_NO_CACHE_HEADERS.each do |k, v| + headers[k] = v + end end def default_headers diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 1a97b39d3ae..1668cf004f8 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -19,7 +19,7 @@ class DashboardController < Dashboard::ApplicationController format.json do load_events - pager_json("events/_events", @events.count) + pager_json('events/_events', @events.count { |event| event.visible_to_user?(current_user) }) end end end @@ -37,6 +37,7 @@ class DashboardController < Dashboard::ApplicationController @events = EventCollection .new(projects, offset: params[:offset].to_i, filter: event_filter) .to_a + .map(&:present) Events::RenderService.new(current_user).execute(@events) end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 0953ca96317..958dc27984f 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -91,7 +91,7 @@ class GroupsController < Groups::ApplicationController format.json do load_events - pager_json("events/_events", @events.count) + pager_json("events/_events", @events.count { |event| event.visible_to_user?(current_user) }) end end end @@ -209,8 +209,9 @@ class GroupsController < Groups::ApplicationController .includes(:namespace) @events = EventCollection - .new(projects, offset: params[:offset].to_i, filter: event_filter, groups: groups) - .to_a + .new(projects, offset: params[:offset].to_i, filter: event_filter, groups: groups) + .to_a + .map(&:present) Events::RenderService .new(current_user) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 47d6fb67108..6ade27a6d76 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -119,7 +119,7 @@ class ProjectsController < Projects::ApplicationController format.html format.json do load_events - pager_json('events/_events', @events.count) + pager_json('events/_events', @events.count { |event| event.visible_to_user?(current_user) }) end end end @@ -340,6 +340,7 @@ class ProjectsController < Projects::ApplicationController @events = EventCollection .new(projects, offset: params[:offset].to_i, filter: event_filter) .to_a + .map(&:present) Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) end diff --git a/app/models/project.rb b/app/models/project.rb index 3f6c2d6a448..544abf16e64 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2333,6 +2333,10 @@ class Project < ApplicationRecord end end + def template_source? + false + end + private def closest_namespace_setting(name) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 3a16f7dc239..c93a19bdc3d 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -21,6 +21,14 @@ class BasePolicy < DeclarativePolicy::Base with_options scope: :user, score: 0 condition(:deactivated) { @user&.deactivated? } + desc "User email is unconfirmed or user account is locked" + with_options scope: :user, score: 0 + condition(:inactive) do + Feature.enabled?(:inactive_policy_condition, default_enabled: true) && + @user && + !@user&.active_for_authentication? + end + with_options scope: :user, score: 0 condition(:external_user) { @user.nil? || @user.external? } diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index f212bb06bc9..2187c703760 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -36,6 +36,13 @@ class GlobalPolicy < BasePolicy enable :use_slash_commands end + rule { inactive }.policy do + prevent :log_in + prevent :access_api + prevent :access_git + prevent :use_slash_commands + end + rule { blocked | internal }.policy do prevent :log_in prevent :access_api diff --git a/app/presenters/event_presenter.rb b/app/presenters/event_presenter.rb index f31d362d5fa..5657e0b96bc 100644 --- a/app/presenters/event_presenter.rb +++ b/app/presenters/event_presenter.rb @@ -3,6 +3,18 @@ class EventPresenter < Gitlab::View::Presenter::Delegated presents :event + def initialize(subject, **attributes) + super + + @visible_to_user_cache = ActiveSupport::Cache::MemoryStore.new + end + + # Caching `visible_to_user?` method in the presenter beause it might be called multiple times. + def visible_to_user?(user = nil) + @visible_to_user_cache.fetch(user&.id) { super(user) } + end + + # implement cache here def resource_parent_name resource_parent&.full_name || '' end diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index 8344397f67d..38859c1efa4 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -4,6 +4,12 @@ module Projects module ImportExport class ExportService < BaseService def execute(after_export_strategy = nil, options = {}) + unless project.template_source? || can?(current_user, :admin_project, project) + raise ::Gitlab::ImportExport::Error.new( + "User with ID: %s does not have permission to Project %s with ID: %s." % + [current_user.id, project.name, project.id]) + end + @shared = project.import_export_shared save_all! diff --git a/changelogs/unreleased/security-commits-api-last-pipeline-status.yml b/changelogs/unreleased/security-commits-api-last-pipeline-status.yml new file mode 100644 index 00000000000..a68151f9732 --- /dev/null +++ b/changelogs/unreleased/security-commits-api-last-pipeline-status.yml @@ -0,0 +1,5 @@ +--- +title: Disable access to last_pipeline in commits API for users without read permissions +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-dependency-proxy-path-traversal.yml b/changelogs/unreleased/security-dependency-proxy-path-traversal.yml new file mode 100644 index 00000000000..ca0a03e36ab --- /dev/null +++ b/changelogs/unreleased/security-dependency-proxy-path-traversal.yml @@ -0,0 +1,5 @@ +--- +title: Add constraint to group dependency proxy endpoint param +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-email-confirmation-bypass-via-api-ee.yml b/changelogs/unreleased/security-email-confirmation-bypass-via-api-ee.yml new file mode 100644 index 00000000000..8bd2b7a452f --- /dev/null +++ b/changelogs/unreleased/security-email-confirmation-bypass-via-api-ee.yml @@ -0,0 +1,5 @@ +--- +title: Prevent API access for unconfirmed users +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-enforce-permissions-for-event-filter-ee.yml b/changelogs/unreleased/security-enforce-permissions-for-event-filter-ee.yml new file mode 100644 index 00000000000..7d74d6108f8 --- /dev/null +++ b/changelogs/unreleased/security-enforce-permissions-for-event-filter-ee.yml @@ -0,0 +1,5 @@ +--- +title: Enforce permission check when counting activity events +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-fix-xss-on-frequent-groups-dropdown.yml b/changelogs/unreleased/security-fix-xss-on-frequent-groups-dropdown.yml new file mode 100644 index 00000000000..970708fe8d5 --- /dev/null +++ b/changelogs/unreleased/security-fix-xss-on-frequent-groups-dropdown.yml @@ -0,0 +1,5 @@ +--- +title: Fix xss on frequent groups dropdown +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-project_export_service_permission_check.yml b/changelogs/unreleased/security-project_export_service_permission_check.yml new file mode 100644 index 00000000000..a38aaabfc9b --- /dev/null +++ b/changelogs/unreleased/security-project_export_service_permission_check.yml @@ -0,0 +1,5 @@ +--- +title: ImportExport::ExportService to require admin_project permission +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-remove-caching-from-api-project-raw-endpoint.yml b/changelogs/unreleased/security-remove-caching-from-api-project-raw-endpoint.yml new file mode 100644 index 00000000000..308a618da89 --- /dev/null +++ b/changelogs/unreleased/security-remove-caching-from-api-project-raw-endpoint.yml @@ -0,0 +1,5 @@ +--- +title: Disable caching of repository/files/:file_path/raw API endpoint +merge_request: +author: +type: security diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 63a7fdfa3ab..9dcf9b015aa 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -154,7 +154,7 @@ module API not_found! 'Commit' unless commit - present commit, with: Entities::CommitDetail, stats: params[:stats] + present commit, with: Entities::CommitDetail, stats: params[:stats], current_user: current_user end desc 'Get the diff for a specific commit of a project' do diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 76963777566..48551c92a12 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -476,8 +476,18 @@ module API class CommitDetail < Commit expose :stats, using: Entities::CommitStats, if: :stats expose :status - expose :last_pipeline, using: 'API::Entities::PipelineBasic' expose :project_id + + expose :last_pipeline do |commit, options| + pipeline = commit.last_pipeline if can_read_pipeline? + ::API::Entities::PipelineBasic.represent(pipeline, options) + end + + private + + def can_read_pipeline? + Ability.allowed?(options[:current_user], :read_pipeline, object.last_pipeline) + end end class CommitSignature < Grape::Entity diff --git a/lib/api/files.rb b/lib/api/files.rb index 0b438fb5bbc..feed22d188c 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -127,6 +127,7 @@ module API get ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS do assign_file_vars! + no_cache_headers set_http_headers(blob_data) send_git_blob @repo, @blob diff --git a/lib/api/helpers/headers_helpers.rb b/lib/api/helpers/headers_helpers.rb index 7553af9d156..908c57bb04e 100644 --- a/lib/api/helpers/headers_helpers.rb +++ b/lib/api/helpers/headers_helpers.rb @@ -3,6 +3,8 @@ module API module Helpers module HeadersHelpers + include Gitlab::NoCacheHeaders + def set_http_headers(header_data) header_data.each do |key, value| if value.is_a?(Enumerable) @@ -12,6 +14,12 @@ module API header "X-Gitlab-#{key.to_s.split('_').collect(&:capitalize).join('-')}", value.to_s end end + + def no_cache_headers + DEFAULT_GITLAB_NO_CACHE_HEADERS.each do |k, v| + header k, v + end + end end end end diff --git a/lib/gitlab/no_cache_headers.rb b/lib/gitlab/no_cache_headers.rb new file mode 100644 index 00000000000..f80ca2c1369 --- /dev/null +++ b/lib/gitlab/no_cache_headers.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Gitlab + module NoCacheHeaders + DEFAULT_GITLAB_NO_CACHE_HEADERS = { + 'Cache-Control' => "#{ActionDispatch::Http::Cache::Response::DEFAULT_CACHE_CONTROL}, no-store, no-cache", + 'Pragma' => 'no-cache', # HTTP 1.0 compatibility + 'Expires' => 'Fri, 01 Jan 1990 00:00:00 GMT' + }.freeze + + def no_cache_headers + raise "#no_cache_headers is not implemented for this object" + end + end +end diff --git a/qa/qa/specs/features/api/3_create/repository/files_spec.rb b/qa/qa/specs/features/api/3_create/repository/files_spec.rb index f6f020da472..dc471128dae 100644 --- a/qa/qa/specs/features/api/3_create/repository/files_spec.rb +++ b/qa/qa/specs/features/api/3_create/repository/files_spec.rb @@ -59,5 +59,48 @@ module QA a_hash_including(message: '202 Accepted') ) end + + describe 'raw file access' do + let(:svg_file) do + <<-SVG + <?xml version="1.0" standalone="no"?> + <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> + + <svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg"> + <polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400"/> + <script type="text/javascript"> + alert("surprise"); + </script> + </svg> + SVG + end + + it 'sets no-cache headers as expected' do + create_project_request = Runtime::API::Request.new(@api_client, '/projects') + post create_project_request.url, path: project_name, name: project_name + + create_file_request = Runtime::API::Request.new(@api_client, "/projects/#{sanitized_project_path}/repository/files/test.svg") + post create_file_request.url, branch: 'master', content: svg_file, commit_message: 'Add test.svg' + + get_file_request = Runtime::API::Request.new(@api_client, "/projects/#{sanitized_project_path}/repository/files/test.svg/raw", ref: 'master') + + 3.times do + response = get get_file_request.url + + # Subsequent responses aren't cached, so headers should match from + # request to request, especially a 200 response rather than a 304 + # (indicating a cached response.) Further, :content_disposition + # should include `attachment` for all responses. + # + expect(response.headers[:cache_control]).to include("no-store") + expect(response.headers[:cache_control]).to include("no-cache") + expect(response.headers[:pragma]).to eq("no-cache") + expect(response.headers[:expires]).to eq("Fri, 01 Jan 1990 00:00:00 GMT") + expect(response.headers[:content_disposition]).to include("attachment") + expect(response.headers[:content_disposition]).not_to include("inline") + expect(response.headers[:content_type]).to include("image/svg+xml") + end + end + end end end diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index a733c3ecaa1..305419efe96 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -23,6 +23,47 @@ describe DashboardController do end end + describe "GET activity as JSON" do + render_views + + let(:user) { create(:user) } + let(:project) { create(:project, :public, issues_access_level: ProjectFeature::PRIVATE) } + + before do + create(:event, :created, project: project, target: create(:issue)) + + sign_in(user) + + request.cookies[:event_filter] = 'all' + end + + context 'when user has permission to see the event' do + before do + project.add_developer(user) + end + + it 'returns count' do + get :activity, params: { format: :json } + + expect(json_response['count']).to eq(1) + end + end + + context 'when user has no permission to see the event' do + it 'filters out invisible event' do + get :activity, params: { format: :json } + + expect(json_response['html']).to include(_('No activities found')) + end + + it 'filters out invisible event when calculating the count' do + get :activity, params: { format: :json } + + expect(json_response['count']).to eq(0) + end + end + end + it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 2ed2b319298..ddfd2b424e7 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -47,7 +47,7 @@ describe GroupsController do it 'assigns events for all the projects in the group', :sidekiq_might_not_need_inline do subject - expect(assigns(:events)).to contain_exactly(event) + expect(assigns(:events).map(&:id)).to contain_exactly(event.id) end end end @@ -119,12 +119,12 @@ describe GroupsController do describe 'GET #activity' do render_views - before do - sign_in(user) - project - end - context 'as json' do + before do + sign_in(user) + project + end + it 'includes events from all projects in group and subgroups', :sidekiq_might_not_need_inline do 2.times do project = create(:project, group: group) @@ -141,6 +141,31 @@ describe GroupsController do expect(assigns(:projects).limit_value).to be_nil end end + + context 'when user has no permission to see the event' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + let(:project_with_restricted_access) do + create(:project, :public, issues_access_level: ProjectFeature::PRIVATE, group: group) + end + + before do + create(:event, project: project) + create(:event, :created, project: project_with_restricted_access, target: create(:issue)) + + group.add_guest(user) + + sign_in(user) + end + + it 'filters out invisible event' do + get :activity, params: { id: group.to_param }, format: :json + + expect(json_response['count']).to eq(1) + end + end end describe 'POST #create' do diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 5259c612bbd..35b422445e5 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -64,6 +64,46 @@ describe ProjectsController do end end + describe "GET #activity as JSON" do + render_views + + let(:project) { create(:project, :public, issues_access_level: ProjectFeature::PRIVATE) } + + before do + create(:event, :created, project: project, target: create(:issue)) + + sign_in(user) + + request.cookies[:event_filter] = 'all' + end + + context 'when user has permission to see the event' do + before do + project.add_developer(user) + end + + it 'returns count' do + get :activity, params: { namespace_id: project.namespace, id: project, format: :json } + + expect(json_response['count']).to eq(1) + end + end + + context 'when user has no permission to see the event' do + it 'filters out invisible event' do + get :activity, params: { namespace_id: project.namespace, id: project, format: :json } + + expect(json_response['html']).to eq("\n") + end + + it 'filters out invisible event when calculating the count' do + get :activity, params: { namespace_id: project.namespace, id: project, format: :json } + + expect(json_response['count']).to eq(0) + end + end + end + describe "GET show" do context "user not project member" do before do diff --git a/spec/javascripts/frequent_items/utils_spec.js b/spec/javascripts/frequent_items/utils_spec.js index cd27d79b29a..0c4d747d2d8 100644 --- a/spec/javascripts/frequent_items/utils_spec.js +++ b/spec/javascripts/frequent_items/utils_spec.js @@ -1,5 +1,10 @@ import bp from '~/breakpoints'; -import { isMobile, getTopFrequentItems, updateExistingFrequentItem } from '~/frequent_items/utils'; +import { + isMobile, + getTopFrequentItems, + updateExistingFrequentItem, + sanitizeItem, +} from '~/frequent_items/utils'; import { HOUR_IN_MS, FREQUENT_ITEMS } from '~/frequent_items/constants'; import { mockProject, unsortedFrequentItems, sortedFrequentItems } from './mock_data'; @@ -86,4 +91,16 @@ describe('Frequent Items utils spec', () => { expect(result.frequency).toBe(mockedProject.frequency); }); }); + + describe('sanitizeItem', () => { + it('strips HTML tags for name and namespace', () => { + const input = { + name: '<br><b>test</b>', + namespace: '<br>test', + id: 1, + }; + + expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 }); + }); + }); }); diff --git a/spec/lib/gitlab/no_cache_headers_spec.rb b/spec/lib/gitlab/no_cache_headers_spec.rb new file mode 100644 index 00000000000..f011b55006e --- /dev/null +++ b/spec/lib/gitlab/no_cache_headers_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::NoCacheHeaders do + class NoCacheTester + include Gitlab::NoCacheHeaders + end + + describe "#no_cache_headers" do + subject { NoCacheTester.new } + + it "raises a RuntimeError" do + expect { subject.no_cache_headers }.to raise_error(RuntimeError) + end + end +end diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index f715ecae347..d227c018694 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -141,6 +141,34 @@ describe GlobalPolicy do it { is_expected.to be_allowed(:access_api) } end end + + context 'inactive user' do + before do + current_user.update!(confirmed_at: nil, confirmation_sent_at: 5.days.ago) + end + + context 'when within the confirmation grace period' do + before do + allow(User).to receive(:allow_unconfirmed_access_for).and_return(10.days) + end + + it { is_expected.to be_allowed(:access_api) } + end + + context 'when confirmation grace period is expired' do + before do + allow(User).to receive(:allow_unconfirmed_access_for).and_return(2.days) + end + + it { is_expected.not_to be_allowed(:access_api) } + end + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:access_api) + end + end end describe 'receive notifications' do @@ -202,6 +230,20 @@ describe GlobalPolicy do it { is_expected.not_to be_allowed(:access_git) } end + describe 'inactive user' do + before do + current_user.update!(confirmed_at: nil) + end + + it { is_expected.not_to be_allowed(:access_git) } + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:access_git) + end + end + context 'when terms are enforced' do before do enforce_terms @@ -298,6 +340,20 @@ describe GlobalPolicy do it { is_expected.not_to be_allowed(:use_slash_commands) } end + describe 'inactive user' do + before do + current_user.update!(confirmed_at: nil) + end + + it { is_expected.not_to be_allowed(:use_slash_commands) } + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:use_slash_commands) + end + end + context 'when access locked' do before do current_user.lock_access! diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index d8da1c001b0..e390f3945a9 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -8,6 +8,7 @@ describe API::Commits do let(:user) { create(:user) } let(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let(:developer) { create(:user).tap { |u| project.add_developer(u) } } let(:project) { create(:project, :repository, creator: user, path: 'my.project') } let(:branch_with_dot) { project.repository.find_branch('ends-with.json') } let(:branch_with_slash) { project.repository.find_branch('improve/awesome') } @@ -964,6 +965,56 @@ describe API::Commits do end end + shared_examples_for 'ref with pipeline' do + let!(:pipeline) do + project + .ci_pipelines + .create!(source: :push, ref: 'master', sha: commit.sha, protected: false) + end + + it 'includes status as "created" and a last_pipeline object' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(json_response['status']).to eq('created') + expect(json_response['last_pipeline']['id']).to eq(pipeline.id) + expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref) + expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha) + expect(json_response['last_pipeline']['status']).to eq(pipeline.status) + end + + context 'when pipeline succeeds' do + before do + pipeline.update!(status: 'success') + end + + it 'includes a "success" status' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(json_response['status']).to eq('success') + end + end + end + + shared_examples_for 'ref with unaccessible pipeline' do + let!(:pipeline) do + project + .ci_pipelines + .create!(source: :push, ref: 'master', sha: commit.sha, protected: false) + end + + it 'does not include last_pipeline' do + get api(route, current_user) + + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(response).to have_gitlab_http_status(200) + expect(json_response['last_pipeline']).to be_nil + end + end + context 'when stat param' do let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}" } @@ -993,6 +1044,15 @@ describe API::Commits do let(:project) { create(:project, :public, :repository) } it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' + + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + end + + it_behaves_like 'ref with unaccessible pipeline' + end end context 'when unauthenticated', 'and project is private' do @@ -1006,6 +1066,17 @@ describe API::Commits do let(:current_user) { user } it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' + + context 'when builds are disabled' do + before do + project + .project_feature + .update!(builds_access_level: ProjectFeature::DISABLED) + end + + it_behaves_like 'ref with unaccessible pipeline' + end context 'when branch contains a dot' do let(:commit) { project.repository.commit(branch_with_dot.name) } @@ -1041,35 +1112,53 @@ describe API::Commits do it_behaves_like 'ref commit' end end + end - context 'when the ref has a pipeline' do - let!(:pipeline) { project.ci_pipelines.create(source: :push, ref: 'master', sha: commit.sha, protected: false) } + context 'when authenticated', 'as a developer' do + let(:current_user) { developer } - it 'includes a "created" status' do - get api(route, current_user) + it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' - expect(response).to have_gitlab_http_status(200) - expect(response).to match_response_schema('public_api/v4/commit/detail') - expect(json_response['status']).to eq('created') - expect(json_response['last_pipeline']['id']).to eq(pipeline.id) - expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref) - expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha) - expect(json_response['last_pipeline']['status']).to eq(pipeline.status) + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) end - context 'when pipeline succeeds' do - before do - pipeline.update(status: 'success') - end + it_behaves_like 'ref with pipeline' + end + end - it 'includes a "success" status' do - get api(route, current_user) + context 'when authenticated', 'as a guest' do + let(:current_user) { guest } - expect(response).to have_gitlab_http_status(200) - expect(response).to match_response_schema('public_api/v4/commit/detail') - expect(json_response['status']).to eq('success') - end + it_behaves_like '403 response' do + let(:request) { get api(route, guest) } + let(:message) { '403 Forbidden' } + end + end + + context 'when authenticated', 'as a non member' do + let(:current_user) { create(:user) } + + it_behaves_like '403 response' do + let(:request) { get api(route, guest) } + let(:message) { '403 Forbidden' } + end + end + + context 'when authenticated', 'as non_member and project is public' do + let(:current_user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'ref with pipeline' + + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) end + + it_behaves_like 'ref with unaccessible pipeline' end end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index ab915af8ab0..efad443de3f 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -447,6 +447,18 @@ describe API::Files do expect(response).to have_gitlab_http_status(200) end + it 'sets no-cache headers' do + url = route('.gitignore') + "/raw" + expect(Gitlab::Workhorse).to receive(:send_git_blob) + + get api(url, current_user), params: params + + expect(response.headers["Cache-Control"]).to include("no-store") + expect(response.headers["Cache-Control"]).to include("no-cache") + expect(response.headers["Pragma"]).to eq("no-cache") + expect(response.headers["Expires"]).to eq("Fri, 01 Jan 1990 00:00:00 GMT") + end + context 'when mandatory params are not given' do it_behaves_like '400 response' do let(:request) { get api(route("any%2Ffile"), current_user) } diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index 8d7b3fa3c09..ce03756a19a 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -30,26 +30,40 @@ describe 'OAuth tokens' do end end - context "when user is blocked" do - it "does not create an access token" do - user = create(:user) + shared_examples 'does not create an access token' do + let(:user) { create(:user) } + + it { expect(response).to have_gitlab_http_status(401) } + end + + context 'when user is blocked' do + before do user.block request_oauth_token(user) - - expect(response).to have_gitlab_http_status(401) end + + include_examples 'does not create an access token' end - context "when user is ldap_blocked" do - it "does not create an access token" do - user = create(:user) + context 'when user is ldap_blocked' do + before do user.ldap_block request_oauth_token(user) + end - expect(response).to have_gitlab_http_status(401) + include_examples 'does not create an access token' + end + + context 'when user account is not confirmed' do + before do + user.update!(confirmed_at: nil) + + request_oauth_token(user) end + + include_examples 'does not create an access token' end end end diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb index a557e61da78..0069b73d310 100644 --- a/spec/services/projects/import_export/export_service_spec.rb +++ b/spec/services/projects/import_export/export_service_spec.rb @@ -10,6 +10,10 @@ describe Projects::ImportExport::ExportService do let(:service) { described_class.new(project, user) } let!(:after_export_strategy) { Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy.new } + before do + project.add_maintainer(user) + end + it 'saves the version' do expect(Gitlab::ImportExport::VersionSaver).to receive(:new).and_call_original @@ -133,5 +137,18 @@ describe Projects::ImportExport::ExportService do expect(service).not_to receive(:execute_after_export_action) end end + + context 'when user does not have admin_project permission' do + let!(:another_user) { create(:user) } + + subject(:service) { described_class.new(project, another_user) } + + it 'fails' do + expected_message = + "User with ID: %s does not have permission to Project %s with ID: %s." % + [another_user.id, project.name, project.id] + expect { service.execute }.to raise_error(Gitlab::ImportExport::Error).with_message(expected_message) + end + end end end |