diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-12-07 20:24:11 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-12-07 20:24:11 +0000 |
commit | 1b517a5a19c4aafc6fa6d738b0ee7c1e4a2cce36 (patch) | |
tree | 11bb99aa913b3a18342818e15a71856e214caee6 | |
parent | 1b6a590b197788a06a1ff726ea61630a49b10412 (diff) | |
parent | f508fb8a043a4b1f996cb7ec8bba198e5b5986f5 (diff) | |
download | gitlab-ce-1b517a5a19c4aafc6fa6d738b0ee7c1e4a2cce36.tar.gz |
Merge remote-tracking branch 'dev/13-6-stable' into 13-6-stable
36 files changed, 649 insertions, 56 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b0c4b07275..d2ecaa19b49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.6.2 (2020-12-07) + +### Security (10 changes) + +- Validate zoom links to start with https only. !1055 +- Require at least 3 characters when searching for project in the Explore page. +- Do not show emails of users in confirmation page. +- Forbid setting a gitlabUserList strategy to a list from another project. +- Fix mermaid resource consumption in GFM fields. +- Ensure group and project memberships are not leaked via API for users with private profiles. +- GraphQL User: do not expose email if set to private. +- Filter search parameter to prevent data leaks. +- Do not expose starred projects of users with private profile via API. +- Do not show starred & contributed projects of users with private profile. + + ## 13.6.1 (2020-11-23) ### Fixed (5 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 156557438e1..cf51d24272d 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.6.1
\ No newline at end of file +13.6.2
\ No newline at end of file @@ -1 +1 @@ -13.6.1
\ No newline at end of file +13.6.2
\ No newline at end of file diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index 233c5f84340..602f156dbf0 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -18,7 +18,13 @@ import { __, sprintf } from '~/locale'; // // This is an arbitrary number; Can be iterated upon when suitable. -const MAX_CHAR_LIMIT = 5000; +const MAX_CHAR_LIMIT = 2000; +// Max # of mermaid blocks that can be rendered in a page. +const MAX_MERMAID_BLOCK_LIMIT = 50; +// Keep a map of mermaid blocks we've already rendered. +const elsProcessingMap = new WeakMap(); +let renderedMermaidBlocks = 0; + let mermaidModule = {}; function importMermaidModule() { @@ -110,13 +116,22 @@ function renderMermaids($els) { let renderedChars = 0; $els.each((i, el) => { + // Skipping all the elements which we've already queued in requestIdleCallback + if (elsProcessingMap.has(el)) { + return; + } + const { source } = fixElementSource(el); /** - * Restrict the rendering to a certain amount of character to - * prevent mermaidjs from hanging up the entire thread and - * causing a DoS. + * Restrict the rendering to a certain amount of character + * and mermaid blocks to prevent mermaidjs from hanging + * up the entire thread and causing a DoS. */ - if ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT) { + if ( + (source && source.length > MAX_CHAR_LIMIT) || + renderedChars > MAX_CHAR_LIMIT || + renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT + ) { const html = ` <div class="alert gl-alert gl-alert-warning alert-dismissible lazy-render-mermaid-container js-lazy-render-mermaid-container fade show" role="alert"> <div> @@ -146,8 +161,13 @@ function renderMermaids($els) { } renderedChars += source.length; + renderedMermaidBlocks += 1; + + const requestId = window.requestIdleCallback(() => { + renderMermaidEl(el); + }); - renderMermaidEl(el); + elsProcessingMap.set(el, requestId); }); }) .catch(err => { diff --git a/app/controllers/explore/projects_controller.rb b/app/controllers/explore/projects_controller.rb index 42795e418a4..7a485eebfe3 100644 --- a/app/controllers/explore/projects_controller.rb +++ b/app/controllers/explore/projects_controller.rb @@ -8,6 +8,8 @@ class Explore::ProjectsController < Explore::ApplicationController include SortingHelper include SortingPreference + MIN_SEARCH_LENGTH = 3 + before_action :set_non_archived_param before_action :set_sorting @@ -72,7 +74,7 @@ class Explore::ProjectsController < Explore::ApplicationController def load_projects load_project_counts - projects = ProjectsFinder.new(current_user: current_user, params: params).execute + projects = ProjectsFinder.new(current_user: current_user, params: params.merge(minimum_search_length: MIN_SEARCH_LENGTH)).execute projects = preload_associations(projects) projects = projects.page(params[:page]).without_count diff --git a/app/controllers/projects/feature_flags_controller.rb b/app/controllers/projects/feature_flags_controller.rb index e9d450a6ce3..9142f769b28 100644 --- a/app/controllers/projects/feature_flags_controller.rb +++ b/app/controllers/projects/feature_flags_controller.rb @@ -77,7 +77,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController end else respond_to do |format| - format.json { render_error_json(result[:message]) } + format.json { render_error_json(result[:message], result[:http_status]) } end end end @@ -167,8 +167,8 @@ class Projects::FeatureFlagsController < Projects::ApplicationController render json: feature_flag_json(feature_flag), status: :ok end - def render_error_json(messages) + def render_error_json(messages, status = :bad_request) render json: { message: messages }, - status: :bad_request + status: status end end diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 4b21edc98d5..c92b3457640 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -144,7 +144,6 @@ class SearchController < ApplicationController payload[:metadata] ||= {} payload[:metadata]['meta.search.group_id'] = params[:group_id] payload[:metadata]['meta.search.project_id'] = params[:project_id] - payload[:metadata]['meta.search.search'] = params[:search] payload[:metadata]['meta.search.scope'] = params[:scope] payload[:metadata]['meta.search.filters.confidential'] = params[:confidential] payload[:metadata]['meta.search.filters.state'] = params[:state] diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 672f36dedc0..05573255066 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -19,7 +19,7 @@ class UsersController < ApplicationController prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) } before_action :user, except: [:exists, :suggests] before_action :authorize_read_user_profile!, - only: [:calendar, :calendar_activities, :groups, :projects, :contributed_projects, :starred_projects, :snippets] + only: [:calendar, :calendar_activities, :groups, :projects, :contributed, :starred, :snippets] feature_category :users diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 14b84d0bfa6..05dc69ebff6 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -18,6 +18,7 @@ # personal: boolean # search: string # search_namespaces: boolean +# minimum_search_length: int # non_archived: boolean # archived: 'only' or boolean # min_access_level: integer @@ -182,6 +183,9 @@ class ProjectsFinder < UnionFinder def by_search(items) params[:search] ||= params[:name] + + return items.none if params[:search].present? && params[:minimum_search_length].present? && params[:search].length < params[:minimum_search_length].to_i + items.optionally_search(params[:search], include_namespace: params[:search_namespaces].present?) end diff --git a/app/finders/starred_projects_finder.rb b/app/finders/starred_projects_finder.rb index fcb469d1d17..e209960c471 100644 --- a/app/finders/starred_projects_finder.rb +++ b/app/finders/starred_projects_finder.rb @@ -1,11 +1,22 @@ # frozen_string_literal: true class StarredProjectsFinder < ProjectsFinder + include Gitlab::Allowable + def initialize(user, params: {}, current_user: nil) + @user = user + super( params: params, current_user: current_user, project_ids_relation: user.starred_projects.select(:id) ) end + + def execute + # Do not show starred projects if the user has a private profile. + return Project.none unless can?(current_user, :read_user_profile, @user) + + super + end end diff --git a/app/graphql/types/user_type.rb b/app/graphql/types/user_type.rb index 11c5369f726..783a0d8425a 100644 --- a/app/graphql/types/user_type.rb +++ b/app/graphql/types/user_type.rb @@ -19,7 +19,7 @@ module Types field :state, Types::UserStateEnum, null: false, description: 'State of the user' field :email, GraphQL::STRING_TYPE, null: true, - description: 'User email' + description: 'User email', method: :public_email field :avatar_url, GraphQL::STRING_TYPE, null: true, description: "URL of the user's avatar" field :web_url, GraphQL::STRING_TYPE, null: false, @@ -30,8 +30,7 @@ module Types resolver: Resolvers::TodoResolver, description: 'Todos of the user' field :group_memberships, Types::GroupMemberType.connection_type, null: true, - description: 'Group memberships of the user', - method: :group_members + description: 'Group memberships of the user' field :group_count, GraphQL::INT_TYPE, null: true, resolver: Resolvers::Users::GroupCountResolver, description: 'Group count for the user', @@ -39,8 +38,7 @@ module Types field :status, Types::UserStatusType, null: true, description: 'User status' field :project_memberships, Types::ProjectMemberType.connection_type, null: true, - description: 'Project memberships of the user', - method: :project_members + description: 'Project memberships of the user' field :starred_projects, Types::ProjectType.connection_type, null: true, description: 'Projects starred by the user', resolver: Resolvers::UserStarredProjectsResolver diff --git a/app/models/operations/feature_flags/user_list.rb b/app/models/operations/feature_flags/user_list.rb index 3e492eaa892..ec109bde0eb 100644 --- a/app/models/operations/feature_flags/user_list.rb +++ b/app/models/operations/feature_flags/user_list.rb @@ -28,6 +28,11 @@ module Operations fuzzy_search(query, [:name], use_minimum_char_limit: false) end + def self.belongs_to?(project_id, user_list_ids) + uniq_ids = user_list_ids.uniq + where(id: uniq_ids, project_id: project_id).count == uniq_ids.count + end + private def ensure_no_associated_strategies diff --git a/app/presenters/user_presenter.rb b/app/presenters/user_presenter.rb index f201b36346f..0028e6d9ef0 100644 --- a/app/presenters/user_presenter.rb +++ b/app/presenters/user_presenter.rb @@ -2,4 +2,18 @@ class UserPresenter < Gitlab::View::Presenter::Delegated presents :user + + def group_memberships + should_be_private? ? GroupMember.none : user.group_members + end + + def project_memberships + should_be_private? ? ProjectMember.none : user.project_members + end + + private + + def should_be_private? + !can?(current_user, :read_user_profile, user) + end end diff --git a/app/services/feature_flags/update_service.rb b/app/services/feature_flags/update_service.rb index ed5e2e794b4..d956d4b3357 100644 --- a/app/services/feature_flags/update_service.rb +++ b/app/services/feature_flags/update_service.rb @@ -10,6 +10,7 @@ module FeatureFlags def execute(feature_flag) return error('Access Denied', 403) unless can_update?(feature_flag) + return error('Not Found', 404) unless valid_user_list_ids?(feature_flag, user_list_ids(params)) ActiveRecord::Base.transaction do feature_flag.assign_attributes(params) @@ -87,5 +88,15 @@ module FeatureFlags def can_update?(feature_flag) Ability.allowed?(current_user, :update_feature_flag, feature_flag) end + + def user_list_ids(params) + params.fetch(:strategies_attributes, []) + .select { |s| s[:user_list_id].present? } + .map { |s| s[:user_list_id] } + end + + def valid_user_list_ids?(feature_flag, user_list_ids) + user_list_ids.empty? || ::Operations::FeatureFlags::UserList.belongs_to?(feature_flag.project_id, user_list_ids) + end end end diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index 97c56b84434..7cfedc2233a 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -22,7 +22,7 @@ module Todos # if at least reporter, all entities including confidential issues can be accessed return if user_has_reporter_access? - remove_confidential_issue_todos + remove_confidential_resource_todos if entity.private? remove_project_todos @@ -40,7 +40,7 @@ module Todos end end - def remove_confidential_issue_todos + def remove_confidential_resource_todos Todo .for_target(confidential_issues.select(:id)) .for_type(Issue.name) @@ -133,3 +133,5 @@ module Todos end end end + +Todos::Destroy::EntityLeaveService.prepend_if_ee('EE::Todos::Destroy::EntityLeaveService') diff --git a/app/validators/zoom_url_validator.rb b/app/validators/zoom_url_validator.rb index dc4ca6b9501..e0f8e4e34a2 100644 --- a/app/validators/zoom_url_validator.rb +++ b/app/validators/zoom_url_validator.rb @@ -5,8 +5,13 @@ # Custom validator for zoom urls # class ZoomUrlValidator < ActiveModel::EachValidator + ALLOWED_SCHEMES = %w(https).freeze + def validate_each(record, attribute, value) - return if Gitlab::ZoomLinkExtractor.new(value).links.size == 1 + links_count = Gitlab::ZoomLinkExtractor.new(value).links.size + valid = Gitlab::UrlSanitizer.valid?(value, allowed_schemes: ALLOWED_SCHEMES) + + return if links_count == 1 && valid record.errors.add(:url, 'must contain one valid Zoom URL') end diff --git a/app/views/devise/confirmations/new.html.haml b/app/views/devise/confirmations/new.html.haml index 49112ed6cd5..770a29a629e 100644 --- a/app/views/devise/confirmations/new.html.haml +++ b/app/views/devise/confirmations/new.html.haml @@ -6,7 +6,7 @@ = render "devise/shared/error_messages", resource: resource .form-group = f.label :email - = f.email_field :email, class: "form-control", required: true, title: 'Please provide a valid email address.' + = f.email_field :email, class: "form-control", required: true, title: 'Please provide a valid email address.', value: nil .clearfix = f.submit "Resend", class: 'gl-button btn btn-success' diff --git a/app/views/explore/projects/_projects.html.haml b/app/views/explore/projects/_projects.html.haml index 4275f76c046..b2154f71082 100644 --- a/app/views/explore/projects/_projects.html.haml +++ b/app/views/explore/projects/_projects.html.haml @@ -1 +1,5 @@ -= render 'shared/projects/list', projects: projects, user: current_user, explore_page: true, pipeline_status: Feature.enabled?(:dashboard_pipeline_status, default_enabled: true) +- if params[:name].present? && params[:name].size < Explore::ProjectsController::MIN_SEARCH_LENGTH + .nothing-here-block + %h5= _('Enter at least three characters to search') +- else + = render 'shared/projects/list', projects: projects, user: current_user, explore_page: true, pipeline_status: Feature.enabled?(:dashboard_pipeline_status, default_enabled: true) diff --git a/config/application.rb b/config/application.rb index e8aebec086b..3981ba348ae 100644 --- a/config/application.rb +++ b/config/application.rb @@ -137,6 +137,7 @@ module Gitlab encrypted_key import_url elasticsearch_url + search otp_attempt sentry_dsn trace diff --git a/config/feature_categories.yml b/config/feature_categories.yml index fb261377532..205d3ee5851 100644 --- a/config/feature_categories.yml +++ b/config/feature_categories.yml @@ -45,6 +45,7 @@ - dynamic_application_security_testing - editor_extension - epics +- epic_tracking - error_tracking - feature_flags - foundations diff --git a/db/post_migrate/20201109114603_schedule_remove_inaccessible_epic_todos.rb b/db/post_migrate/20201109114603_schedule_remove_inaccessible_epic_todos.rb new file mode 100644 index 00000000000..13d12675a28 --- /dev/null +++ b/db/post_migrate/20201109114603_schedule_remove_inaccessible_epic_todos.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class ScheduleRemoveInaccessibleEpicTodos < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INTERVAL = 2.minutes + BATCH_SIZE = 10 + MIGRATION = 'RemoveInaccessibleEpicTodos' + + disable_ddl_transaction! + + class Epic < ActiveRecord::Base + include EachBatch + end + + def up + return unless Gitlab.ee? + + relation = Epic.where(confidential: true) + + queue_background_migration_jobs_by_range_at_intervals( + relation, MIGRATION, INTERVAL, batch_size: BATCH_SIZE) + end + + def down + # no-op + end +end diff --git a/db/schema_migrations/20201109114603 b/db/schema_migrations/20201109114603 new file mode 100644 index 00000000000..c1df2223dbd --- /dev/null +++ b/db/schema_migrations/20201109114603 @@ -0,0 +1 @@ +ae8034ec52df47ce2ce3397715dd18347e4d297a963c17c7b26321f414dfa632
\ No newline at end of file diff --git a/doc/user/todos.md b/doc/user/todos.md index 061d81618a1..69c0736bdff 100644 --- a/doc/user/todos.md +++ b/doc/user/todos.md @@ -64,7 +64,7 @@ To-do item triggers aren't affected by [GitLab notification email settings](prof NOTE: **Note:** When a user no longer has access to a resource related to a to-do item (such as -an issue, merge request, project, or group), for security reasons GitLab +an issue, merge request, epic, project, or group), for security reasons GitLab deletes any related to-do items within the next hour. Deletion is delayed to prevent data loss, in the case where a user's access is accidentally revoked. diff --git a/lib/gitlab/background_migration/remove_inaccessible_epic_todos.rb b/lib/gitlab/background_migration/remove_inaccessible_epic_todos.rb new file mode 100644 index 00000000000..74c48b237cc --- /dev/null +++ b/lib/gitlab/background_migration/remove_inaccessible_epic_todos.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # rubocop:disable Style/Documentation + class RemoveInaccessibleEpicTodos + def perform(start_id, stop_id) + end + end + end +end + +Gitlab::BackgroundMigration::RemoveInaccessibleEpicTodos.prepend_if_ee('EE::Gitlab::BackgroundMigration::RemoveInaccessibleEpicTodos') diff --git a/spec/controllers/confirmations_controller_spec.rb b/spec/controllers/confirmations_controller_spec.rb new file mode 100644 index 00000000000..49a39f257fe --- /dev/null +++ b/spec/controllers/confirmations_controller_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ConfirmationsController do + include DeviseHelpers + + before do + set_devise_mapping(context: @request) + end + + describe '#show' do + render_views + + subject { get :show, params: { confirmation_token: confirmation_token } } + + context 'user is already confirmed' do + let_it_be_with_reload(:user) { create(:user, :unconfirmed) } + let(:confirmation_token) { user.confirmation_token } + + before do + user.confirm + subject + end + + it 'renders `new`' do + expect(response).to render_template(:new) + end + + it 'displays an error message' do + expect(response.body).to include('Email was already confirmed, please try signing in') + end + + it 'does not display the email of the user' do + expect(response.body).not_to include(user.email) + end + end + + context 'user accesses the link after the expiry of confirmation token has passed' do + let_it_be_with_reload(:user) { create(:user, :unconfirmed) } + let(:confirmation_token) { user.confirmation_token } + + before do + allow(Devise).to receive(:confirm_within).and_return(1.day) + + travel_to(3.days.from_now) do + subject + end + end + + it 'renders `new`' do + expect(response).to render_template(:new) + end + + it 'displays an error message' do + expect(response.body).to include('Email needs to be confirmed within 1 day, please request a new one below') + end + + it 'does not display the email of the user' do + expect(response.body).not_to include(user.email) + end + end + + context 'with an invalid confirmation token' do + let(:confirmation_token) { 'invalid_confirmation_token' } + + before do + subject + end + + it 'renders `new`' do + expect(response).to render_template(:new) + end + + it 'displays an error message' do + expect(response.body).to include('Confirmation token is invalid') + end + end + end +end diff --git a/spec/controllers/projects/feature_flags_controller_spec.rb b/spec/controllers/projects/feature_flags_controller_spec.rb index 96eeb6f239f..1473ec95192 100644 --- a/spec/controllers/projects/feature_flags_controller_spec.rb +++ b/spec/controllers/projects/feature_flags_controller_spec.rb @@ -1511,6 +1511,40 @@ RSpec.describe Projects::FeatureFlagsController do expect(response).to have_gitlab_http_status(:not_found) end + it 'returns not found when trying to update a gitlabUserList strategy with a user list from another project' do + user_list = create(:operations_feature_flag_user_list, project: project, name: 'My List', user_xids: 'user1,user2') + strategy = create(:operations_strategy, feature_flag: new_version_flag, name: 'gitlabUserList', parameters: {}, user_list: user_list) + other_project = create(:project) + other_user_list = create(:operations_feature_flag_user_list, project: other_project, name: 'Other List', user_xids: 'some,one') + + put_request(new_version_flag, strategies_attributes: [{ + id: strategy.id, + user_list_id: other_user_list.id + }]) + + expect(response).to have_gitlab_http_status(:not_found) + expect(strategy.reload.user_list).to eq(user_list) + end + + it 'allows setting multiple gitlabUserList strategies to the same user list' do + user_list_a = create(:operations_feature_flag_user_list, project: project, name: 'My List A', user_xids: 'user1,user2') + user_list_b = create(:operations_feature_flag_user_list, project: project, name: 'My List B', user_xids: 'user3,user4') + strategy_a = create(:operations_strategy, feature_flag: new_version_flag, name: 'gitlabUserList', parameters: {}, user_list: user_list_a) + strategy_b = create(:operations_strategy, feature_flag: new_version_flag, name: 'gitlabUserList', parameters: {}, user_list: user_list_a) + + put_request(new_version_flag, strategies_attributes: [{ + id: strategy_a.id, + user_list_id: user_list_b.id + }, { + id: strategy_b.id, + user_list_id: user_list_b.id + }]) + + expect(response).to have_gitlab_http_status(:ok) + expect(strategy_a.reload.user_list).to eq(user_list_b) + expect(strategy_b.reload.user_list).to eq(user_list_b) + end + it 'updates an existing strategy' do strategy = create(:operations_strategy, feature_flag: new_version_flag, name: 'default', parameters: {}) diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index afebc6982c1..bbd39fd4c83 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -272,7 +272,7 @@ RSpec.describe SearchController do expect(last_payload[:metadata]['meta.search.group_id']).to eq('123') expect(last_payload[:metadata]['meta.search.project_id']).to eq('456') - expect(last_payload[:metadata]['meta.search.search']).to eq('hello world') + expect(last_payload[:metadata]).not_to have_key('meta.search.search') expect(last_payload[:metadata]['meta.search.scope']).to eq('issues') expect(last_payload[:metadata]['meta.search.force_search_results']).to eq('true') expect(last_payload[:metadata]['meta.search.filters.confidential']).to eq('true') diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index bec4b24484a..2e57a901319 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -247,32 +247,99 @@ RSpec.describe UsersController do describe 'GET #contributed' do let(:project) { create(:project, :public) } - let(:current_user) { create(:user) } + + subject do + get :contributed, params: { username: author.username }, format: format + end before do - sign_in(current_user) + sign_in(user) project.add_developer(public_user) project.add_developer(private_user) + create(:push_event, project: project, author: author) + + subject end - context 'with public profile' do + shared_examples_for 'renders contributed projects' do it 'renders contributed projects' do - create(:push_event, project: project, author: public_user) + expect(assigns[:contributed_projects]).not_to be_empty + expect(response).to have_gitlab_http_status(:ok) + end + end - get :contributed, params: { username: public_user.username } + %i(html json).each do |format| + context "format: #{format}" do + let(:format) { format } - expect(assigns[:contributed_projects]).not_to be_empty + context 'with public profile' do + let(:author) { public_user } + + it_behaves_like 'renders contributed projects' + end + + context 'with private profile' do + let(:author) { private_user } + + it 'returns 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'with a user that has the ability to read private profiles', :enable_admin_mode do + let(:user) { create(:admin) } + + it_behaves_like 'renders contributed projects' + end + end + end + end + end + + describe 'GET #starred' do + let(:project) { create(:project, :public) } + + subject do + get :starred, params: { username: author.username }, format: format + end + + before do + author.toggle_star(project) + + sign_in(user) + subject + end + + shared_examples_for 'renders starred projects' do + it 'renders starred projects' do + expect(response).to have_gitlab_http_status(:ok) + expect(assigns[:starred_projects]).not_to be_empty end end - context 'with private profile' do - it 'does not render contributed projects' do - create(:push_event, project: project, author: private_user) + %i(html json).each do |format| + context "format: #{format}" do + let(:format) { format } + + context 'with public profile' do + let(:author) { public_user } + + it_behaves_like 'renders starred projects' + end + + context 'with private profile' do + let(:author) { private_user } + + it 'returns 404' do + expect(response).to have_gitlab_http_status(:not_found) + end - get :contributed, params: { username: private_user.username } + context 'with a user that has the ability to read private profiles', :enable_admin_mode do + let(:user) { create(:admin) } - expect(assigns[:contributed_projects]).to be_empty + it_behaves_like 'renders starred projects' + end + end end end end diff --git a/spec/features/explore/user_explores_projects_spec.rb b/spec/features/explore/user_explores_projects_spec.rb index bf4d6c946e1..c082ff1fb0c 100644 --- a/spec/features/explore/user_explores_projects_spec.rb +++ b/spec/features/explore/user_explores_projects_spec.rb @@ -47,6 +47,14 @@ RSpec.describe 'User explores projects' do end end + shared_examples 'minimum search length' do + it 'shows a prompt to enter a longer search term', :js do + fill_in 'name', with: 'z' + + expect(page).to have_content('Enter at least three characters to search') + end + end + context 'when viewing public projects' do before do visit(explore_projects_path) @@ -54,6 +62,7 @@ RSpec.describe 'User explores projects' do include_examples 'shows public and internal projects' include_examples 'empty search results' + include_examples 'minimum search length' end context 'when viewing most starred projects' do @@ -63,6 +72,7 @@ RSpec.describe 'User explores projects' do include_examples 'shows public and internal projects' include_examples 'empty search results' + include_examples 'minimum search length' end context 'when viewing trending projects' do @@ -76,6 +86,7 @@ RSpec.describe 'User explores projects' do include_examples 'shows public projects' include_examples 'empty search results' + include_examples 'minimum search length' end end end diff --git a/spec/features/markdown/mermaid_spec.rb b/spec/features/markdown/mermaid_spec.rb index bdb549326fa..58314a49c4b 100644 --- a/spec/features/markdown/mermaid_spec.rb +++ b/spec/features/markdown/mermaid_spec.rb @@ -19,6 +19,9 @@ RSpec.describe 'Mermaid rendering', :js do visit project_issue_path(project, issue) + wait_for_requests + wait_for_mermaid + %w[A B C D].each do |label| expect(page).to have_selector('svg text', text: label) end @@ -39,6 +42,7 @@ RSpec.describe 'Mermaid rendering', :js do visit project_issue_path(project, issue) wait_for_requests + wait_for_mermaid expected = '<text style=""><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>' expect(page.html.scan(expected).count).to be(4) @@ -65,6 +69,9 @@ RSpec.describe 'Mermaid rendering', :js do visit project_issue_path(project, issue) + wait_for_requests + wait_for_mermaid + page.within('.description') do expect(page).to have_selector('svg') expect(page).to have_selector('pre.mermaid') @@ -92,6 +99,9 @@ RSpec.describe 'Mermaid rendering', :js do visit project_issue_path(project, issue) + wait_for_requests + wait_for_mermaid + page.within('.description') do page.find('summary').click svg = page.find('svg.mermaid') @@ -118,6 +128,9 @@ RSpec.describe 'Mermaid rendering', :js do visit project_issue_path(project, issue) + wait_for_requests + wait_for_mermaid + expect(page).to have_css('svg.mermaid[style*="max-width"][width="100%"]') end @@ -147,6 +160,7 @@ RSpec.describe 'Mermaid rendering', :js do end wait_for_requests + wait_for_mermaid find('.js-lazy-render-mermaid').click @@ -156,4 +170,55 @@ RSpec.describe 'Mermaid rendering', :js do expect(page).not_to have_selector('.js-lazy-render-mermaid-container') end end + + it 'does not render more than 50 mermaid blocks', :js, quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/234081' } do + graph_edges = "A-->B;B-->A;" + + description = <<~MERMAID + ```mermaid + graph LR + #{graph_edges} + ``` + MERMAID + + description *= 51 + + project = create(:project, :public) + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + wait_for_requests + wait_for_mermaid + + page.within('.description') do + expect(page).to have_selector('svg') + + expect(page).to have_selector('.lazy-alert-shown') + + expect(page).to have_selector('.js-lazy-render-mermaid-container') + end + end +end + +def wait_for_mermaid + run_idle_callback = <<~RUN_IDLE_CALLBACK + window.requestIdleCallback(() => { + window.__CAPYBARA_IDLE_CALLBACK_EXEC__ = 1; + }) + RUN_IDLE_CALLBACK + + page.evaluate_script(run_idle_callback) + + Timeout.timeout(Capybara.default_max_wait_time) do + loop until finished_rendering? + end +end + +def finished_rendering? + check_idle_callback = <<~CHECK_IDLE_CALLBACK + window.__CAPYBARA_IDLE_CALLBACK_EXEC__ + CHECK_IDLE_CALLBACK + page.evaluate_script(check_idle_callback) == 1 end diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 2d712bd44ce..57977fb69b4 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -161,6 +161,29 @@ RSpec.describe ProjectsFinder, :do_not_mock_admin_mode do it { is_expected.to eq([public_project]) } end + describe 'filter by search with minimum search length' do + context 'when search term is shorter than minimum length' do + let(:params) { { search: 'C', minimum_search_length: 3 } } + + it { is_expected.to be_empty } + end + + context 'when search term is longer than minimum length' do + let(:project) { create(:project, :public, group: group, name: 'test_project') } + let(:params) { { search: 'test', minimum_search_length: 3 } } + + it { is_expected.to eq([project]) } + end + + context 'when minimum length is invalid' do + let(:params) { { search: 'C', minimum_search_length: 'x' } } + + it 'ignores the minimum length param' do + is_expected.to eq([public_project]) + end + end + end + describe 'filter by group name' do let(:params) { { name: group.name, search_namespaces: true } } diff --git a/spec/finders/starred_projects_finder_spec.rb b/spec/finders/starred_projects_finder_spec.rb index 15d4ae52ddd..f5d3314021d 100644 --- a/spec/finders/starred_projects_finder_spec.rb +++ b/spec/finders/starred_projects_finder_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe StarredProjectsFinder do let(:project1) { create(:project, :public, :empty_repo) } let(:project2) { create(:project, :public, :empty_repo) } - let(:other_project) { create(:project, :public, :empty_repo) } + let(:private_project) { create(:project, :private, :empty_repo) } let(:user) { create(:user) } let(:other_user) { create(:user) } @@ -13,6 +13,9 @@ RSpec.describe StarredProjectsFinder do before do user.toggle_star(project1) user.toggle_star(project2) + + private_project.add_maintainer(user) + user.toggle_star(private_project) end describe '#execute' do @@ -20,22 +23,56 @@ RSpec.describe StarredProjectsFinder do subject { finder.execute } - describe 'as same user' do - let(:current_user) { user } + context 'user has a public profile' do + describe 'as same user' do + let(:current_user) { user } - it { is_expected.to contain_exactly(project1, project2) } - end + it { is_expected.to contain_exactly(project1, project2, private_project) } + end + + describe 'as other user' do + let(:current_user) { other_user } - describe 'as other user' do - let(:current_user) { other_user } + it { is_expected.to contain_exactly(project1, project2) } + end - it { is_expected.to contain_exactly(project1, project2) } + describe 'as no user' do + let(:current_user) { nil } + + it { is_expected.to contain_exactly(project1, project2) } + end end - describe 'as no user' do - let(:current_user) { nil } + context 'user has a private profile' do + before do + user.update!(private_profile: true) + end + + describe 'as same user' do + let(:current_user) { user } + + it { is_expected.to contain_exactly(project1, project2, private_project) } + end + + describe 'as other user' do + context 'user does not have access to view the private profile' do + let(:current_user) { other_user } + + it { is_expected.to be_empty } + end + + context 'user has access to view the private profile', :enable_admin_mode do + let(:current_user) { create(:admin) } + + it { is_expected.to contain_exactly(project1, project2, private_project) } + end + end + + describe 'as no user' do + let(:current_user) { nil } - it { is_expected.to contain_exactly(project1, project2) } + it { is_expected.to be_empty } + end end end end diff --git a/spec/requests/api/graphql/user/starred_projects_query_spec.rb b/spec/requests/api/graphql/user/starred_projects_query_spec.rb index 8a1bd3d172f..b098058a735 100644 --- a/spec/requests/api/graphql/user/starred_projects_query_spec.rb +++ b/spec/requests/api/graphql/user/starred_projects_query_spec.rb @@ -70,4 +70,31 @@ RSpec.describe 'Getting starredProjects of the user' do ) end end + + context 'the user has a private profile' do + before do + user.update!(private_profile: true) + post_graphql(query, current_user: current_user) + end + + context 'the current user does not have access to view the private profile of the user' do + let(:current_user) { create(:user) } + + it 'finds no projects' do + expect(starred_projects).to be_empty + end + end + + context 'the current user has access to view the private profile of the user' do + let(:current_user) { create(:admin) } + + it 'finds all projects starred by the user, which the current user has access to' do + expect(starred_projects).to contain_exactly( + a_hash_including('id' => global_id_of(project_a)), + a_hash_including('id' => global_id_of(project_b)), + a_hash_including('id' => global_id_of(project_c)) + ) + end + end + end end diff --git a/spec/requests/api/graphql/user_query_spec.rb b/spec/requests/api/graphql/user_query_spec.rb index 738e120549e..8c45a67cb0f 100644 --- a/spec/requests/api/graphql/user_query_spec.rb +++ b/spec/requests/api/graphql/user_query_spec.rb @@ -82,7 +82,7 @@ RSpec.describe 'getting user information' do 'username' => presenter.username, 'webUrl' => presenter.web_url, 'avatarUrl' => presenter.avatar_url, - 'email' => presenter.email + 'email' => presenter.public_email )) expect(graphql_data['user']['status']).to match( @@ -250,7 +250,7 @@ RSpec.describe 'getting user information' do context 'the user is private' do before do - user.update(private_profile: true) + user.update!(private_profile: true) post_graphql(query, current_user: current_user) end @@ -260,6 +260,50 @@ RSpec.describe 'getting user information' do it_behaves_like 'a working graphql query' end + context 'we request the groupMemberships' do + let_it_be(:membership_a) { create(:group_member, user: user) } + let(:group_memberships) { graphql_data_at(:user, :group_memberships, :nodes) } + let(:user_fields) { 'groupMemberships { nodes { id } }' } + + it_behaves_like 'a working graphql query' + + it 'cannot be found' do + expect(group_memberships).to be_empty + end + + context 'the current user is the user' do + let(:current_user) { user } + + it 'can be found' do + expect(group_memberships).to include( + a_hash_including('id' => global_id_of(membership_a)) + ) + end + end + end + + context 'we request the projectMemberships' do + let_it_be(:membership_a) { create(:project_member, user: user) } + let(:project_memberships) { graphql_data_at(:user, :project_memberships, :nodes) } + let(:user_fields) { 'projectMemberships { nodes { id } }' } + + it_behaves_like 'a working graphql query' + + it 'cannot be found' do + expect(project_memberships).to be_empty + end + + context 'the current user is the user' do + let(:current_user) { user } + + it 'can be found' do + expect(project_memberships).to include( + a_hash_including('id' => global_id_of(membership_a)) + ) + end + end + end + context 'we request the authoredMergeRequests' do let(:user_fields) { 'authoredMergeRequests { nodes { id } }' } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 4a792fc218d..234ac1778fd 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1255,13 +1255,46 @@ RSpec.describe API::Projects do expect(json_response['message']).to eq('404 User Not Found') end - it 'returns projects filtered by user' do - get api("/users/#{user3.id}/starred_projects/", user) + context 'with a public profile' do + it 'returns projects filtered by user' do + get api("/users/#{user3.id}/starred_projects/", user) - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.map { |project| project['id'] }).to contain_exactly(project.id, project2.id, project3.id) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }) + .to contain_exactly(project.id, project2.id, project3.id) + end + end + + context 'with a private profile' do + before do + user3.update!(private_profile: true) + user3.reload + end + + context 'user does not have access to view the private profile' do + it 'returns no projects' do + get api("/users/#{user3.id}/starred_projects/", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response).to be_empty + end + end + + context 'user has access to view the private profile' do + it 'returns projects filtered by user' do + get api("/users/#{user3.id}/starred_projects/", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }) + .to contain_exactly(project.id, project2.id, project3.id) + end + end end end diff --git a/spec/validators/zoom_url_validator_spec.rb b/spec/validators/zoom_url_validator_spec.rb new file mode 100644 index 00000000000..7d5c94bc249 --- /dev/null +++ b/spec/validators/zoom_url_validator_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ZoomUrlValidator do + let(:zoom_meeting) { build(:zoom_meeting) } + + describe 'validations' do + context 'when zoom link starts with https' do + it 'passes validation' do + zoom_meeting.url = 'https://zoom.us/j/123456789' + + expect(zoom_meeting.valid?).to eq(true) + expect(zoom_meeting.errors).to be_empty + end + end + + shared_examples 'zoom link does not start with https' do |url| + it 'fails validation' do + zoom_meeting.url = url + expect(zoom_meeting.valid?).to eq(false) + + expect(zoom_meeting.errors).to be_present + expect(zoom_meeting.errors.first[1]).to eq 'must contain one valid Zoom URL' + end + end + + context 'when zoom link does not start with https' do + include_examples 'zoom link does not start with https', 'http://zoom.us/j/123456789' + + context 'when zoom link does not start with a scheme' do + include_examples 'zoom link does not start with https', 'testinghttp://zoom.us/j/123456789' + end + end + end +end |