From e129eff88309eca18f3902afd710e2e07393fe45 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 5 Jul 2022 18:08:43 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .gitlab/ci/frontend.gitlab-ci.yml | 6 + .gitlab/ci/rules.gitlab-ci.yml | 2 - .gitlab/ci/static-analysis.gitlab-ci.yml | 12 +- .../rails/helper_instance_variable.yml | 1 - .rubocop_todo/rspec/expect_in_hook.yml | 2 - .rubocop_todo/rspec/instance_variable.yml | 1 - .../rspec/repeated_example_group_body.yml | 7 +- .../repeated_example_group_description.yml | 18 +- .rubocop_todo/style/case_like_if.yml | 1 - .rubocop_todo/style/empty_method.yml | 1 - Gemfile | 2 +- Gemfile.lock | 4 +- app/assets/javascripts/mirrors/ssh_mirror.js | 2 +- .../oauth_application_ids_controller.rb | 9 +- .../jira_connect/subscriptions_controller.rb | 8 + .../namespace_storage_limit_alert_helper.rb | 9 - app/helpers/todos_helper.rb | 3 +- app/models/group.rb | 18 +- app/models/members/project_member.rb | 10 +- app/models/merge_request_diff.rb | 51 ++ app/models/merge_request_diff_file.rb | 34 ++ app/models/project.rb | 2 +- app/models/project_team.rb | 18 +- app/models/ssh_host_key.rb | 10 +- .../issues/related_branches_service.rb | 2 +- app/services/members/create_service.rb | 2 +- app/services/members/creator_service.rb | 55 +- app/services/members/invite_service.rb | 4 +- .../members/projects/creator_service.rb | 2 +- .../quick_actions/interpret_service.rb | 42 +- .../resource_access_tokens/create_service.rb | 2 +- app/views/layouts/_page.html.haml | 1 - app/views/layouts/group.html.haml | 2 +- app/views/layouts/project.html.haml | 2 +- .../projects/mirrors/_ssh_host_keys.html.haml | 2 +- config/application.rb | 10 + .../development/approval_rules_pagination.yml | 8 + ...externally_stored_diffs_caching_export.yml | 8 + .../refactor_mr_widgets_extensions.yml | 2 +- config/routes.rb | 2 + db/fixtures/development/06_teams.rb | 2 +- doc/development/fips_compliance.md | 10 + .../group/settings/group_access_tokens.md | 2 +- lib/gitlab/ci/variables/builder.rb | 1 - .../instance_administrators/create_group.rb | 4 +- .../json/streaming_serializer.rb | 18 + .../import_export/project/import_export.yml | 2 +- .../import_export/project/relation_factory.rb | 2 +- lib/gitlab/quick_actions/users_extractor.rb | 94 ++++ lib/tasks/gitlab/bulk_add_permission.rake | 12 +- locale/gitlab.pot | 6 + qa/qa.rb | 4 + qa/qa/scenario/template.rb | 2 + .../groups/variables_controller_spec.rb | 2 +- .../available_namespaces_controller_spec.rb | 6 +- .../projects/issues_controller_spec.rb | 2 +- .../projects/mirrors_controller_spec.rb | 2 +- spec/controllers/projects_controller_spec.rb | 2 +- spec/features/admin/admin_groups_spec.rb | 8 +- spec/features/admin/admin_projects_spec.rb | 4 +- .../multipart_invalid_uploads_spec.rb | 2 +- spec/features/issuables/issuable_list_spec.rb | 2 +- .../filtered_search/visual_tokens_spec.rb | 4 +- ...r_creates_branch_and_merge_request_spec.rb | 2 +- .../user_sees_deployment_widget_spec.rb | 2 +- ...r_adds_member_with_expiration_date_spec.rb | 4 +- ..._interacts_with_auto_devops_banner_spec.rb | 2 +- .../user_sees_collaboration_links_spec.rb | 2 +- spec/features/users/login_spec.rb | 4 +- .../finders/autocomplete/users_finder_spec.rb | 2 +- spec/finders/joined_groups_finder_spec.rb | 2 +- .../packages/conan/package_finder_spec.rb | 2 +- .../packages/group_packages_finder_spec.rb | 4 +- .../ide/components/new_dropdown/modal_spec.js | 492 ++++++++++++------ .../ci/job_token_scope_resolver_spec.rb | 4 +- .../container_repositories_resolver_spec.rb | 2 +- .../types/ci/job_token_scope_type_spec.rb | 4 +- spec/helpers/diff_helper_spec.rb | 53 +- ...mespace_storage_limit_alert_helper_spec.rb | 11 - spec/helpers/projects_helper_spec.rb | 2 +- spec/helpers/todos_helper_spec.rb | 10 + .../reference_parser/snippet_parser_spec.rb | 2 +- .../cycle_analytics/records_fetcher_spec.rb | 6 +- spec/lib/gitlab/ci/variables/builder_spec.rb | 10 +- .../create_group_spec.rb | 2 +- .../json/streaming_serializer_spec.rb | 23 + .../import_export/members_mapper_spec.rb | 6 +- .../project/tree_restorer_spec.rb | 2 +- .../quick_actions/users_extractor_spec.rb | 93 ++++ spec/lib/gitlab/user_access_spec.rb | 34 +- spec/mailers/notify_spec.rb | 8 +- spec/models/ci/build_spec.rb | 25 +- spec/models/group_spec.rb | 94 ++-- spec/models/members/group_member_spec.rb | 6 +- spec/models/members/project_member_spec.rb | 10 +- spec/models/merge_request_diff_file_spec.rb | 144 ++++- spec/models/merge_request_diff_spec.rb | 97 ++++ spec/models/merge_request_spec.rb | 2 +- spec/models/project_spec.rb | 4 +- spec/models/project_team_spec.rb | 10 +- spec/models/ssh_host_key_spec.rb | 41 +- spec/models/user_spec.rb | 30 +- spec/policies/environment_policy_spec.rb | 8 +- .../root_storage_statistics_policy_spec.rb | 2 +- spec/policies/project_policy_spec.rb | 6 +- .../project_statistics_policy_spec.rb | 2 +- .../api/ci/runner/jobs_request_post_spec.rb | 8 - spec/requests/api/events_spec.rb | 2 +- spec/requests/api/graphql/ci/runner_spec.rb | 2 +- .../container_repository_details_spec.rb | 2 +- .../group/container_repositories_spec.rb | 2 +- .../group/dependency_proxy_blobs_spec.rb | 2 +- .../dependency_proxy_group_setting_spec.rb | 2 +- .../dependency_proxy_image_ttl_policy_spec.rb | 2 +- .../group/dependency_proxy_manifests_spec.rb | 2 +- .../project/container_repositories_spec.rb | 2 +- .../project/packages_cleanup_policy_spec.rb | 2 +- spec/requests/api/group_variables_spec.rb | 2 +- spec/requests/api/invitations_spec.rb | 26 +- .../oauth_application_ids_controller_spec.rb | 53 ++ .../subscriptions_controller_spec.rb | 29 ++ spec/requests/openid_connect_spec.rb | 14 +- .../projects/environments_controller_spec.rb | 2 +- spec/serializers/member_user_entity_spec.rb | 4 +- spec/services/groups/destroy_service_spec.rb | 10 +- spec/services/groups/update_service_spec.rb | 12 +- .../issues/related_branches_service_spec.rb | 2 +- spec/services/members/creator_service_spec.rb | 4 +- .../members/groups/creator_service_spec.rb | 6 +- .../members/projects/creator_service_spec.rb | 6 +- spec/services/notification_service_spec.rb | 6 +- .../services/projects/destroy_service_spec.rb | 2 +- spec/services/projects/fork_service_spec.rb | 12 +- spec/services/projects/update_service_spec.rb | 2 +- .../quick_actions/interpret_service_spec.rb | 57 +- spec/services/todo_service_spec.rb | 4 +- spec/support/helpers/project_helpers.rb | 2 +- .../finders/issues_finder_shared_examples.rb | 2 +- .../models/member_shared_examples.rb | 78 +-- .../api/conan_packages_shared_examples.rb | 2 +- .../deploy_tokens/_form.html.haml_spec.rb | 2 +- ...sallow_two_factor_for_group_worker_spec.rb | 2 +- .../remove_expired_members_worker_spec.rb | 4 +- workhorse/go.mod | 2 +- workhorse/go.sum | 6 +- .../lsif_transformer/parser/code_hover.go | 4 +- 146 files changed, 1589 insertions(+), 637 deletions(-) delete mode 100644 app/helpers/namespace_storage_limit_alert_helper.rb create mode 100644 config/feature_flags/development/approval_rules_pagination.yml create mode 100644 config/feature_flags/development/externally_stored_diffs_caching_export.yml create mode 100644 lib/gitlab/quick_actions/users_extractor.rb delete mode 100644 spec/helpers/namespace_storage_limit_alert_helper_spec.rb create mode 100644 spec/lib/gitlab/quick_actions/users_extractor_spec.rb create mode 100644 spec/requests/jira_connect/subscriptions_controller_spec.rb diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index dc246897fa2..a7c103bae13 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -186,6 +186,12 @@ graphql-schema-dump: - tmp/tests/graphql/gitlab_schema.graphql - tmp/tests/graphql/gitlab_schema.json +graphql-schema-dump as-if-foss: + extends: + - graphql-schema-dump + - .frontend:rules:eslint-as-if-foss + - .as-if-foss + .frontend-test-base: extends: - .default-retry diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index fb4016a10f0..6c3909c9e78 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -828,9 +828,7 @@ .frontend:rules:eslint-as-if-foss: rules: - !reference [".strict-ee-only-rules", rules] - # We already have `static-analysis as-if-foss` which already runs `lint:eslint:all` if the `pipeline:run-as-if-foss` label is set. - <<: *if-merge-request-labels-as-if-foss - when: never - <<: *if-merge-request changes: *frontend-patterns diff --git a/.gitlab/ci/static-analysis.gitlab-ci.yml b/.gitlab/ci/static-analysis.gitlab-ci.yml index 0fda3872500..cb3a9706a18 100644 --- a/.gitlab/ci/static-analysis.gitlab-ci.yml +++ b/.gitlab/ci/static-analysis.gitlab-ci.yml @@ -49,7 +49,7 @@ static-verification-with-database: variables: SETUP_DB: "true" -generate-apollo-graphl-schema: +generate-apollo-graphql-schema: extends: - .static-analysis-base - .frontend:rules:default-frontend-jobs @@ -66,12 +66,19 @@ generate-apollo-graphl-schema: paths: - "${GRAPHQL_SCHEMA_APOLLO_FILE}" +generate-apollo-graphql-schema as-if-foss: + extends: + - generate-apollo-graphql-schema + - .frontend:rules:eslint-as-if-foss + - .as-if-foss + needs: ['graphql-schema-dump as-if-foss'] + eslint: extends: - .static-analysis-base - .yarn-cache - .frontend:rules:default-frontend-jobs - needs: ['generate-apollo-graphl-schema'] + needs: ['generate-apollo-graphql-schema'] variables: USE_BUNDLE_INSTALL: "false" script: @@ -83,6 +90,7 @@ eslint as-if-foss: - eslint - .frontend:rules:eslint-as-if-foss - .as-if-foss + needs: ['generate-apollo-graphql-schema as-if-foss'] haml-lint foss: extends: diff --git a/.rubocop_todo/rails/helper_instance_variable.yml b/.rubocop_todo/rails/helper_instance_variable.yml index aeed8875f20..006e66ed0b2 100644 --- a/.rubocop_todo/rails/helper_instance_variable.yml +++ b/.rubocop_todo/rails/helper_instance_variable.yml @@ -71,7 +71,6 @@ Rails/HelperInstanceVariable: - 'ee/app/helpers/ee/lock_helper.rb' - 'ee/app/helpers/ee/merge_requests_helper.rb' - 'ee/app/helpers/ee/mirror_helper.rb' - - 'ee/app/helpers/ee/namespace_storage_limit_alert_helper.rb' - 'ee/app/helpers/ee/notes_helper.rb' - 'ee/app/helpers/ee/operations_helper.rb' - 'ee/app/helpers/ee/projects/security/configuration_helper.rb' diff --git a/.rubocop_todo/rspec/expect_in_hook.yml b/.rubocop_todo/rspec/expect_in_hook.yml index 63e2b2f0ed3..56b3f48c670 100644 --- a/.rubocop_todo/rspec/expect_in_hook.yml +++ b/.rubocop_todo/rspec/expect_in_hook.yml @@ -30,7 +30,6 @@ RSpec/ExpectInHook: - 'ee/spec/helpers/billing_plans_helper_spec.rb' - 'ee/spec/helpers/ee/ci/runners_helper_spec.rb' - 'ee/spec/helpers/ee/issues_helper_spec.rb' - - 'ee/spec/helpers/ee/namespace_storage_limit_alert_helper_spec.rb' - 'ee/spec/helpers/ee/welcome_helper_spec.rb' - 'ee/spec/helpers/kerberos_spnego_helper_spec.rb' - 'ee/spec/helpers/vulnerabilities_helper_spec.rb' @@ -95,7 +94,6 @@ RSpec/ExpectInHook: - 'ee/spec/services/projects/update_mirror_service_spec.rb' - 'ee/spec/services/security/findings/cleanup_service_spec.rb' - 'ee/spec/services/upcoming_reconciliations/update_service_spec.rb' - - 'ee/spec/support/shared_examples/controllers/namespace_storage_limit_alert_shared_examples.rb' - 'ee/spec/support/shared_examples/controllers/registrations/projects_controller_shared_examples.rb' - 'ee/spec/support/shared_examples/models/concerns/elastic/cannot_read_cross_project_shared_examples.rb' - 'ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb' diff --git a/.rubocop_todo/rspec/instance_variable.yml b/.rubocop_todo/rspec/instance_variable.yml index 97ed3a7c48d..bbd8902dddb 100644 --- a/.rubocop_todo/rspec/instance_variable.yml +++ b/.rubocop_todo/rspec/instance_variable.yml @@ -14,7 +14,6 @@ RSpec/InstanceVariable: - ee/spec/graphql/types/vulnerability_request_type_spec.rb - ee/spec/graphql/types/vulnerability_response_type_spec.rb - ee/spec/helpers/ee/issuables_helper_spec.rb - - ee/spec/helpers/ee/namespace_storage_limit_alert_helper_spec.rb - ee/spec/helpers/ee/wiki_helper_spec.rb - ee/spec/helpers/notes_helper_spec.rb - ee/spec/helpers/search_helper_spec.rb diff --git a/.rubocop_todo/rspec/repeated_example_group_body.yml b/.rubocop_todo/rspec/repeated_example_group_body.yml index e006396d0d6..6fdeb9a2094 100644 --- a/.rubocop_todo/rspec/repeated_example_group_body.yml +++ b/.rubocop_todo/rspec/repeated_example_group_body.yml @@ -1,8 +1,5 @@ --- RSpec/RepeatedExampleGroupBody: - # Offense count: 143 - # Temporarily disabled due to too many offenses - Enabled: false Exclude: - 'ee/spec/controllers/ee/groups_controller_spec.rb' - 'ee/spec/lib/banzai/filter/references/vulnerability_reference_filters_spec.rb' @@ -18,7 +15,6 @@ RSpec/RepeatedExampleGroupBody: - 'ee/spec/requests/api/graphql/mutations/compliance_management/frameworks/update_spec.rb' - 'ee/spec/requests/groups/security/credentials_controller_spec.rb' - 'ee/spec/services/app_sec/dast/profiles/create_associations_service_spec.rb' - - 'ee/spec/services/groups/sync_service_spec.rb' - 'spec/controllers/groups/registry/repositories_controller_spec.rb' - 'spec/controllers/projects/blob_controller_spec.rb' - 'spec/controllers/projects/graphs_controller_spec.rb' @@ -30,6 +26,7 @@ RSpec/RepeatedExampleGroupBody: - 'spec/features/security/project/private_access_spec.rb' - 'spec/finders/packages/nuget/package_finder_spec.rb' - 'spec/helpers/gitlab_routing_helper_spec.rb' + - 'spec/helpers/groups_helper_spec.rb' - 'spec/lib/api/entities/application_setting_spec.rb' - 'spec/lib/banzai/filter/references/commit_range_reference_filter_spec.rb' - 'spec/lib/banzai/filter/references/commit_reference_filter_spec.rb' @@ -38,9 +35,9 @@ RSpec/RepeatedExampleGroupBody: - 'spec/lib/gitlab/ci/config/entry/release_spec.rb' - 'spec/lib/gitlab/ci/pipeline/seed/build_spec.rb' - 'spec/lib/gitlab/ci/yaml_processor_spec.rb' + - 'spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb' - 'spec/lib/gitlab/empty_search_results_spec.rb' - 'spec/lib/gitlab/import_export/project/sample/relation_factory_spec.rb' - - 'spec/lib/gitlab/import_export/project/tree_restorer_spec.rb' - 'spec/lib/gitlab/lfs/client_spec.rb' - 'spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb' - 'spec/lib/gitlab/sanitizers/exif_spec.rb' diff --git a/.rubocop_todo/rspec/repeated_example_group_description.yml b/.rubocop_todo/rspec/repeated_example_group_description.yml index 562f5602d58..b5c4ac090ae 100644 --- a/.rubocop_todo/rspec/repeated_example_group_description.yml +++ b/.rubocop_todo/rspec/repeated_example_group_description.yml @@ -1,10 +1,8 @@ --- RSpec/RepeatedExampleGroupDescription: - # Offense count: 263 - # Temporarily disabled due to too many offenses - Enabled: false Exclude: - 'ee/spec/finders/merge_trains_finder_spec.rb' + - 'ee/spec/finders/security/vulnerability_reads_finder_spec.rb' - 'ee/spec/graphql/resolvers/vulnerabilities_grade_resolver_spec.rb' - 'ee/spec/graphql/resolvers/vulnerability_severities_count_resolver_spec.rb' - 'ee/spec/helpers/ee/auth_helper_spec.rb' @@ -24,16 +22,15 @@ RSpec/RepeatedExampleGroupDescription: - 'ee/spec/models/software_license_spec.rb' - 'ee/spec/policies/app_sec/fuzzing/coverage/corpus_policy_spec.rb' - 'ee/spec/policies/group_policy_spec.rb' - - 'ee/spec/policies/project_policy_spec.rb' - 'ee/spec/requests/api/graphql/iteration_spec.rb' - 'ee/spec/requests/api/graphql/mutations/iterations/create_spec.rb' - - 'ee/spec/requests/api/graphql/vulnerabilities/sort_spec.rb' - 'ee/spec/requests/groups/security/credentials_controller_spec.rb' + - 'ee/spec/requests/groups/settings/reporting_controller_spec.rb' - 'ee/spec/services/app_sec/dast/profiles/create_associations_service_spec.rb' - 'ee/spec/services/app_sec/dast/site_validations/find_or_create_service_spec.rb' - 'ee/spec/services/audit_event_service_spec.rb' - - 'ee/spec/services/groups/sync_service_spec.rb' - 'ee/spec/services/todo_service_spec.rb' + - 'ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb' - 'ee/spec/support/shared_examples/services/scoped_label_shared_examples.rb' - 'ee/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb' - 'spec/controllers/profiles/notifications_controller_spec.rb' @@ -44,6 +41,7 @@ RSpec/RepeatedExampleGroupDescription: - 'spec/features/merge_request/user_sees_merge_widget_spec.rb' - 'spec/features/projects/jobs_spec.rb' - 'spec/features/projects/new_project_spec.rb' + - 'spec/features/projects/pipelines/legacy_pipeline_spec.rb' - 'spec/features/security/project/private_access_spec.rb' - 'spec/finders/ci/pipelines_for_merge_request_finder_spec.rb' - 'spec/frontend/fixtures/runner.rb' @@ -52,7 +50,6 @@ RSpec/RepeatedExampleGroupDescription: - 'spec/helpers/dropdowns_helper_spec.rb' - 'spec/helpers/gitlab_routing_helper_spec.rb' - 'spec/helpers/namespaces_helper_spec.rb' - - 'spec/initializers/omniauth_spec.rb' - 'spec/lib/banzai/pipeline/gfm_pipeline_spec.rb' - 'spec/lib/gitlab/alert_management/payload/base_spec.rb' - 'spec/lib/gitlab/auth/atlassian/auth_hash_spec.rb' @@ -69,14 +66,15 @@ RSpec/RepeatedExampleGroupDescription: - 'spec/lib/gitlab/ci/pipeline/seed/build_spec.rb' - 'spec/lib/gitlab/ci/yaml_processor_spec.rb' - 'spec/lib/gitlab/data_builder/push_spec.rb' + - 'spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb' - 'spec/lib/gitlab/database_importers/common_metrics/importer_spec.rb' - 'spec/lib/gitlab/git/diff_spec.rb' - 'spec/lib/gitlab/git/push_spec.rb' - 'spec/lib/gitlab/git/repository_spec.rb' - 'spec/lib/gitlab/import_export/project/sample/relation_factory_spec.rb' - - 'spec/lib/gitlab/import_export/project/tree_restorer_spec.rb' - 'spec/lib/gitlab/kubernetes/rollout_status_spec.rb' - 'spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb' + - 'spec/lib/gitlab/redis/multi_store_spec.rb' - 'spec/lib/gitlab/sanitizers/exif_spec.rb' - 'spec/lib/gitlab/template/finders/global_template_finder_spec.rb' - 'spec/lib/gitlab/usage_data_spec.rb' @@ -93,8 +91,10 @@ RSpec/RepeatedExampleGroupDescription: - 'spec/models/project_spec.rb' - 'spec/models/ssh_host_key_spec.rb' - 'spec/requests/api/files_spec.rb' + - 'spec/requests/api/graphql/ci/runners_spec.rb' - 'spec/requests/api/graphql/project/release_spec.rb' - 'spec/requests/api/group_clusters_spec.rb' + - 'spec/requests/api/internal/base_spec.rb' - 'spec/requests/api/merge_requests_spec.rb' - 'spec/requests/api/notification_settings_spec.rb' - 'spec/requests/api/project_clusters_spec.rb' @@ -105,10 +105,10 @@ RSpec/RepeatedExampleGroupDescription: - 'spec/services/import/github_service_spec.rb' - 'spec/services/merge_requests/refresh_service_spec.rb' - 'spec/services/metrics/dashboard/gitlab_alert_embed_service_spec.rb' - - 'spec/services/resource_access_tokens/create_service_spec.rb' - 'spec/services/verify_pages_domain_service_spec.rb' - 'spec/support/cycle_analytics_helpers/test_generation.rb' - 'spec/support/shared_examples/models/application_setting_shared_examples.rb' + - 'spec/support/shared_examples/models/concerns/limitable_shared_examples.rb' - 'spec/support/shared_examples/requests/api/composer_packages_shared_examples.rb' - 'spec/support/shared_examples/serializers/diff_file_entity_shared_examples.rb' - 'spec/support/shared_examples/services/container_registry_auth_service_shared_examples.rb' diff --git a/.rubocop_todo/style/case_like_if.yml b/.rubocop_todo/style/case_like_if.yml index f1e349fe836..e24ff534e96 100644 --- a/.rubocop_todo/style/case_like_if.yml +++ b/.rubocop_todo/style/case_like_if.yml @@ -24,7 +24,6 @@ Style/CaseLikeIf: - 'ee/app/controllers/concerns/credentials_inventory_actions.rb' - 'ee/app/finders/ee/notes_finder.rb' - 'ee/app/helpers/ee/branches_helper.rb' - - 'ee/app/helpers/ee/namespace_storage_limit_alert_helper.rb' - 'ee/app/services/epics/tree_reorder_service.rb' - 'ee/app/services/merge_request_approval_settings/update_service.rb' - 'ee/lib/gitlab/alert_management/alert_payload_field_extractor.rb' diff --git a/.rubocop_todo/style/empty_method.yml b/.rubocop_todo/style/empty_method.yml index aa3972a0b22..3e6a1efec70 100644 --- a/.rubocop_todo/style/empty_method.yml +++ b/.rubocop_todo/style/empty_method.yml @@ -61,7 +61,6 @@ Style/EmptyMethod: - 'app/controllers/registrations/welcome_controller.rb' - 'app/controllers/search_controller.rb' - 'app/graphql/resolvers/concerns/caching_array_resolver.rb' - - 'app/helpers/namespace_storage_limit_alert_helper.rb' - 'app/helpers/subscribable_banner_helper.rb' - 'app/helpers/users/callouts_helper.rb' - 'app/models/ci/bridge.rb' diff --git a/Gemfile b/Gemfile index 81791a3edab..3aba7d5f933 100644 --- a/Gemfile +++ b/Gemfile @@ -183,7 +183,7 @@ gem 'diffy', '~> 3.3' gem 'diff_match_patch', '~> 0.1.0' # Application server -gem 'rack', '~> 2.2.3.0' +gem 'rack', '~> 2.2.4' # https://github.com/zombocom/rack-timeout/blob/master/README.md#rails-apps-manually gem 'rack-timeout', '~> 0.6.0', require: 'rack/timeout/base' diff --git a/Gemfile.lock b/Gemfile.lock index 3dc9764ccd1..2a8baaee80c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1009,7 +1009,7 @@ GEM pyu-ruby-sasl (0.0.3.3) raabro (1.1.6) racc (1.6.0) - rack (2.2.3.1) + rack (2.2.4) rack-accept (0.4.5) rack (>= 0.4) rack-attack (6.6.1) @@ -1668,7 +1668,7 @@ DEPENDENCIES pry-shell (~> 0.5.0) puma (~> 5.6.2) puma_worker_killer (~> 0.3.1) - rack (~> 2.2.3.0) + rack (~> 2.2.4) rack-attack (~> 6.6.0) rack-cors (~> 1.1.0) rack-oauth2 (~> 1.19.0) diff --git a/app/assets/javascripts/mirrors/ssh_mirror.js b/app/assets/javascripts/mirrors/ssh_mirror.js index e375435436e..eb7c43034a4 100644 --- a/app/assets/javascripts/mirrors/ssh_mirror.js +++ b/app/assets/javascripts/mirrors/ssh_mirror.js @@ -163,7 +163,7 @@ export default class SSHMirror { const $fingerprintsList = this.$hostKeysInformation.find('.js-fingerprints-list'); let fingerprints = ''; sshHostKeys.fingerprints.forEach((fingerprint) => { - const escFingerprints = escape(fingerprint.fingerprint); + const escFingerprints = escape(fingerprint.fingerprint_sha256 || fingerprint.fingerprint); fingerprints += `${escFingerprints}`; }); diff --git a/app/controllers/jira_connect/oauth_application_ids_controller.rb b/app/controllers/jira_connect/oauth_application_ids_controller.rb index 05c23210da2..a84b47f4c8b 100644 --- a/app/controllers/jira_connect/oauth_application_ids_controller.rb +++ b/app/controllers/jira_connect/oauth_application_ids_controller.rb @@ -5,9 +5,10 @@ module JiraConnect feature_category :integrations skip_before_action :authenticate_user! + skip_before_action :verify_authenticity_token def show - if Feature.enabled?(:jira_connect_oauth_self_managed) && jira_connect_application_key.present? + if show_application_id? render json: { application_id: jira_connect_application_key } else head :not_found @@ -16,6 +17,12 @@ module JiraConnect private + def show_application_id? + return if Gitlab.com? + + Feature.enabled?(:jira_connect_oauth_self_managed) && jira_connect_application_key.present? + end + def jira_connect_application_key Gitlab::CurrentSettings.jira_connect_application_key.presence end diff --git a/app/controllers/jira_connect/subscriptions_controller.rb b/app/controllers/jira_connect/subscriptions_controller.rb index 2ba9f8264e1..623113f8413 100644 --- a/app/controllers/jira_connect/subscriptions_controller.rb +++ b/app/controllers/jira_connect/subscriptions_controller.rb @@ -25,6 +25,7 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController before_action :allow_rendering_in_iframe, only: :index before_action :verify_qsh_claim!, only: :index + before_action :allow_self_managed_content_security_policy, only: :index before_action :authenticate_user!, only: :create def index @@ -62,6 +63,13 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController private + def allow_self_managed_content_security_policy + return unless current_jira_installation.instance_url? + + request.content_security_policy.directives['connect-src'] ||= [] + request.content_security_policy.directives['connect-src'] << Gitlab::Utils.append_path(current_jira_installation.instance_url, '/-/jira_connect/oauth_application_ids') + end + def create_service JiraConnectSubscriptions::CreateService.new(current_jira_installation, current_user, namespace_path: params['namespace_path'], jira_user: jira_user) end diff --git a/app/helpers/namespace_storage_limit_alert_helper.rb b/app/helpers/namespace_storage_limit_alert_helper.rb deleted file mode 100644 index ed11f89a7dd..00000000000 --- a/app/helpers/namespace_storage_limit_alert_helper.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -module NamespaceStorageLimitAlertHelper - # Overridden in EE - def display_namespace_storage_limit_alert! - end -end - -NamespaceStorageLimitAlertHelper.prepend_mod_with('NamespaceStorageLimitAlertHelper') diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index c4cfd3b2287..f87125af07d 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -87,7 +87,8 @@ module TodosHelper elsif todo.for_alert? details_project_alert_management_path(todo.project, todo.target) elsif todo.for_issue_or_work_item? - Gitlab::UrlBuilder.build(todo.target, only_path: true) + path_options[:only_path] = true + Gitlab::UrlBuilder.build(todo.target, **path_options) else path = [todo.resource_parent, todo.target] diff --git a/app/models/group.rb b/app/models/group.rb index f5aad6e74ff..5919b1e71bf 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -361,8 +361,8 @@ class Group < Namespace owners.include?(user) end - def add_users(users, access_level, current_user: nil, expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil) - Members::Groups::CreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass + def add_members(users, access_level, current_user: nil, expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil) + Members::Groups::CreatorService.add_members( # rubocop:disable CodeReuse/ServiceClass self, users, access_level, @@ -373,8 +373,8 @@ class Group < Namespace ) end - def add_user(user, access_level, current_user: nil, expires_at: nil, ldap: false, blocking_refresh: true) - Members::Groups::CreatorService.add_user( # rubocop:disable CodeReuse/ServiceClass + def add_member(user, access_level, current_user: nil, expires_at: nil, ldap: false, blocking_refresh: true) + Members::Groups::CreatorService.add_member( # rubocop:disable CodeReuse/ServiceClass self, user, access_level, @@ -386,23 +386,23 @@ class Group < Namespace end def add_guest(user, current_user = nil) - add_user(user, :guest, current_user: current_user) + add_member(user, :guest, current_user: current_user) end def add_reporter(user, current_user = nil) - add_user(user, :reporter, current_user: current_user) + add_member(user, :reporter, current_user: current_user) end def add_developer(user, current_user = nil) - add_user(user, :developer, current_user: current_user) + add_member(user, :developer, current_user: current_user) end def add_maintainer(user, current_user = nil) - add_user(user, :maintainer, current_user: current_user) + add_member(user, :maintainer, current_user: current_user) end def add_owner(user, current_user = nil) - add_user(user, :owner, current_user: current_user) + add_member(user, :owner, current_user: current_user) end def member?(user, min_access_level = Gitlab::Access::GUEST) diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 791cb6f0dff..d1c7d7c9097 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -21,30 +21,30 @@ class ProjectMember < Member end class << self - # Add users to projects with passed access option + # Add members to projects with passed access option # # access can be an integer representing a access code # or symbol like :maintainer representing role # # Ex. - # add_users_to_projects( + # add_members_to_projects( # project_ids, # user_ids, # ProjectMember::MAINTAINER # ) # - # add_users_to_projects( + # add_members_to_projects( # project_ids, # user_ids, # :maintainer # ) # - def add_users_to_projects(project_ids, users, access_level, current_user: nil, expires_at: nil) + def add_members_to_projects(project_ids, users, access_level, current_user: nil, expires_at: nil) self.transaction do project_ids.each do |project_id| project = Project.find(project_id) - Members::Projects::CreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass + Members::Projects::CreatorService.add_members( # rubocop:disable CodeReuse/ServiceClass project, users, access_level, diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 87afb7a489a..e08b2cc2a7d 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -21,6 +21,10 @@ class MergeRequestDiff < ApplicationRecord # from the database if this sentinel is seen FILES_COUNT_SENTINEL = 2**15 - 1 + # External diff cache key used by diffs export + EXTERNAL_DIFFS_CACHE_TMPDIR = 'project-%{project_id}-external-mr-%{mr_id}-diff-%{id}-cache' + EXTERNAL_DIFF_CACHE_CHUNK_SIZE = 8.megabytes + belongs_to :merge_request manual_inverse_association :merge_request, :merge_request_diff @@ -545,6 +549,28 @@ class MergeRequestDiff < ApplicationRecord merge_request_diff_files.reset end + # Yields locally cached external diff if it's externally stored. + # Used during Project Export to speed up externally + # stored merge request diffs export + def cached_external_diff + return yield(nil) unless stored_externally? + + cache_external_diff unless File.exist?(external_diff_cache_filepath) + + File.open(external_diff_cache_filepath) do |file| + yield(file) + end + end + + def remove_cached_external_diff + Gitlab::Utils.check_path_traversal!(external_diff_cache_dir) + Gitlab::Utils.check_allowed_absolute_path!(external_diff_cache_dir, [Dir.tmpdir]) + + return unless Dir.exist?(external_diff_cache_dir) + + FileUtils.rm_rf(external_diff_cache_dir) + end + private def convert_external_diffs_to_database @@ -791,6 +817,31 @@ class MergeRequestDiff < ApplicationRecord def sort_diffs(diffs) Gitlab::Diff::FileCollectionSorter.new(diffs).sort end + + # Downloads external diff to a temp storage location. + def cache_external_diff + return unless stored_externally? + return if File.exist?(external_diff_cache_filepath) + + Dir.mkdir(external_diff_cache_dir) unless Dir.exist?(external_diff_cache_dir) + + opening_external_diff do |external_diff| + File.open(external_diff_cache_filepath, 'wb') do |file| + file.write(external_diff.read(EXTERNAL_DIFF_CACHE_CHUNK_SIZE)) until external_diff.eof? + end + end + end + + def external_diff_cache_filepath + File.join(external_diff_cache_dir, "diff-#{id}") + end + + def external_diff_cache_dir + File.join( + Dir.tmpdir, + EXTERNAL_DIFFS_CACHE_TMPDIR % { project_id: project.id, mr_id: merge_request_id, id: id } + ) + end end MergeRequestDiff.prepend_mod_with('MergeRequestDiff') diff --git a/app/models/merge_request_diff_file.rb b/app/models/merge_request_diff_file.rb index f7648937c1d..a5c392f6a36 100644 --- a/app/models/merge_request_diff_file.rb +++ b/app/models/merge_request_diff_file.rb @@ -45,4 +45,38 @@ class MergeRequestDiffFile < ApplicationRecord content end end + + # This method is meant to be used during Project Export. + # It is identical to the behaviour in #diff & #utf8_diff with the only + # difference of caching externally stored diffs on local disk in + # temp storage location in order to improve diff export performance. + def diff_export + return utf8_diff unless Feature.enabled?(:externally_stored_diffs_caching_export) + return utf8_diff unless merge_request_diff&.stored_externally? + + content = merge_request_diff.cached_external_diff do |file| + file.seek(external_diff_offset) + + force_encode_utf8(file.read(external_diff_size)) + end + + # See #diff + if binary? + content = begin + content.unpack1('m0') + rescue ArgumentError + content + end + end + + if content.respond_to?(:encoding) + content = encode_utf8(content) + end + + return '' if content.blank? + + content + rescue StandardError + utf8_diff + end end diff --git a/app/models/project.rb b/app/models/project.rb index 6a265dd1a37..f5a69494237 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -460,7 +460,7 @@ class Project < ApplicationRecord delegate :previous_default_branch, :previous_default_branch=, to: :project_setting delegate :name, to: :owner, allow_nil: true, prefix: true delegate :members, to: :team, prefix: true - delegate :add_user, :add_users, to: :team + delegate :add_member, :add_members, to: :team delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_owner, :add_role, to: :team delegate :group_runners_enabled, :group_runners_enabled=, to: :ci_cd_settings, allow_nil: true delegate :root_ancestor, to: :namespace, allow_nil: true diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 97ab5aa2619..5641fbfb867 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -8,23 +8,23 @@ class ProjectTeam end def add_guest(user, current_user: nil) - add_user(user, :guest, current_user: current_user) + add_member(user, :guest, current_user: current_user) end def add_reporter(user, current_user: nil) - add_user(user, :reporter, current_user: current_user) + add_member(user, :reporter, current_user: current_user) end def add_developer(user, current_user: nil) - add_user(user, :developer, current_user: current_user) + add_member(user, :developer, current_user: current_user) end def add_maintainer(user, current_user: nil) - add_user(user, :maintainer, current_user: current_user) + add_member(user, :maintainer, current_user: current_user) end def add_owner(user, current_user: nil) - add_user(user, :owner, current_user: current_user) + add_member(user, :owner, current_user: current_user) end def add_role(user, role, current_user: nil) @@ -43,8 +43,8 @@ class ProjectTeam member end - def add_users(users, access_level, current_user: nil, expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil) - Members::Projects::CreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass + def add_members(users, access_level, current_user: nil, expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil) + Members::Projects::CreatorService.add_members( # rubocop:disable CodeReuse/ServiceClass project, users, access_level, @@ -55,8 +55,8 @@ class ProjectTeam ) end - def add_user(user, access_level, current_user: nil, expires_at: nil) - Members::Projects::CreatorService.add_user( # rubocop:disable CodeReuse/ServiceClass + def add_member(user, access_level, current_user: nil, expires_at: nil) + Members::Projects::CreatorService.add_member( # rubocop:disable CodeReuse/ServiceClass project, user, access_level, diff --git a/app/models/ssh_host_key.rb b/app/models/ssh_host_key.rb index ac7ba9530dd..daa64f4e087 100644 --- a/app/models/ssh_host_key.rb +++ b/app/models/ssh_host_key.rb @@ -12,7 +12,15 @@ class SshHostKey end def as_json(*) - { bits: bits, fingerprint: fingerprint, type: type, index: index } + { bits: bits, type: type, index: index }.merge(fingerprint_data) + end + + private + + def fingerprint_data + data = { fingerprint_sha256: fingerprint_sha256 } + data[:fingerprint] = fingerprint unless Gitlab::FIPS.enabled? + data end end diff --git a/app/services/issues/related_branches_service.rb b/app/services/issues/related_branches_service.rb index 5991caffad1..e2cad85ac6a 100644 --- a/app/services/issues/related_branches_service.rb +++ b/app/services/issues/related_branches_service.rb @@ -38,7 +38,7 @@ module Issues def branches_with_iid_of(issue) branch_name_regex = /\A#{issue.iid}-(?!\d+-stable)/i - project.repository.search_branch_names("#{issue.iid}-*").select do |branch| + project.repository.branch_names.select do |branch| branch.match?(branch_name_regex) end end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 57d9da4cefd..38bebc1d09d 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -84,7 +84,7 @@ module Members end def add_members - @members = source.add_users( + @members = source.add_members( invites, params[:access_level], expires_at: params[:expires_at], diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb index 276093a00a9..217004e634d 100644 --- a/app/services/members/creator_service.rb +++ b/app/services/members/creator_service.rb @@ -13,9 +13,9 @@ module Members Gitlab::Access.sym_options_with_owner end - def add_users( # rubocop:disable Metrics/ParameterLists + def add_members( # rubocop:disable Metrics/ParameterLists source, - users, + invitees, access_level, current_user: nil, expires_at: nil, @@ -24,17 +24,17 @@ module Members ldap: nil, blocking_refresh: nil ) - return [] unless users.present? + return [] unless invitees.present? # If this user is attempting to manage Owner members and doesn't have permission, do not allow return [] if managing_owners?(current_user, access_level) && cannot_manage_owners?(source, current_user) - emails, users, existing_members = parse_users_list(source, users) + emails, users, existing_members = parse_users_list(source, invitees) Member.transaction do - (emails + users).map! do |user| + (emails + users).map! do |invitee| new(source, - user, + invitee, access_level, existing_members: existing_members, current_user: current_user, @@ -48,17 +48,17 @@ module Members end end - def add_user( # rubocop:disable Metrics/ParameterLists + def add_member( # rubocop:disable Metrics/ParameterLists source, - user, + invitee, access_level, current_user: nil, expires_at: nil, ldap: nil, blocking_refresh: nil ) - add_users(source, - [user], + add_members(source, + [invitee], access_level, current_user: current_user, expires_at: expires_at, @@ -113,9 +113,9 @@ module Members end end - def initialize(source, user, access_level, **args) + def initialize(source, invitee, access_level, **args) @source = source - @user = user + @invitee = invitee @access_level = self.class.parsed_access_level(access_level) @args = args end @@ -133,7 +133,7 @@ module Members private delegate :new_record?, to: :member - attr_reader :source, :user, :access_level, :member, :args + attr_reader :source, :invitee, :access_level, :member, :args def assign_member_attributes member.attributes = member_attributes @@ -170,7 +170,7 @@ module Members # Populates the attributes of a member. # # This logic resides in a separate method so that EE can extend this logic, - # without having to patch the `add_user` method directly. + # without having to patch the `add_members` method directly. def member_attributes { created_by: member.created_by || current_user, @@ -241,12 +241,10 @@ module Members end def find_or_build_member - @user = parse_user_param - - @member = if user.is_a?(User) + @member = if invitee.is_a?(User) find_or_initialize_member_by_user else - source.members.build(invite_email: user) + find_or_initialize_member_with_email end @member.blocking_refresh = args[:blocking_refresh] @@ -254,24 +252,23 @@ module Members # This method is used to find users that have been entered into the "Add members" field. # These can be the User objects directly, their IDs, their emails, or new emails to be invited. - def parse_user_param - case user - when User - user - when Integer - # might not return anything - this needs enhancement - User.find_by(id: user) # rubocop:todo CodeReuse/ActiveRecord + def find_or_initialize_member_with_email + if user_by_email + find_or_initialize_member_by_user(user_id: user_by_email.id) else - # must be an email or at least we'll consider it one - source.users_by_emails([user])[user] || user + source.members.build(invite_email: invitee) end end - def find_or_initialize_member_by_user + def user_by_email + source.users_by_emails([invitee])[invitee] + end + + def find_or_initialize_member_by_user(user_id: invitee.id) # We have to use `members_and_requesters` here since the given `members` is modified in the models # to act more like a scope(removing the requested_at members) and therefore ActiveRecord has issues with that # on build and refreshing that relation. - existing_members[user.id] || source.members_and_requesters.build(user_id: user.id) # rubocop:disable CodeReuse/ActiveRecord + existing_members[user_id] || source.members_and_requesters.build(user_id: user_id) # rubocop:disable CodeReuse/ActiveRecord end def ldap diff --git a/app/services/members/invite_service.rb b/app/services/members/invite_service.rb index 1bf209ab79d..6d23a9bc2dc 100644 --- a/app/services/members/invite_service.rb +++ b/app/services/members/invite_service.rb @@ -31,8 +31,8 @@ module Members return if params[:email].blank? - # we need the below due to add_users hitting Members::CreatorService.parse_users_list and ignoring invalid emails - # ideally we wouldn't need this, but we can't really change the add_users method + # we need the below due to add_member hitting Members::CreatorService.parse_users_list and ignoring invalid emails + # ideally we wouldn't need this, but we can't really change the add_members method invalid_emails.each { |email| errors[email] = s_('AddMember|Invite email is invalid') } end diff --git a/app/services/members/projects/creator_service.rb b/app/services/members/projects/creator_service.rb index cde1d0462e8..f45132749f9 100644 --- a/app/services/members/projects/creator_service.rb +++ b/app/services/members/projects/creator_service.rb @@ -32,7 +32,7 @@ module Members end def adding_the_creator_as_owner_in_a_personal_project? - # this condition is reached during testing setup a lot due to use of `.add_user` + # this condition is reached during testing setup a lot due to use of `.add_member` member.project.personal_namespace_holder?(member.user) end diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 4bcb15b2d9c..1d7c5d2c80a 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -69,28 +69,32 @@ module QuickActions Gitlab::QuickActions::Extractor.new(self.class.command_definitions) end + # Find users for commands like /assign + # + # eg. /assign me and @jane and jack def extract_users(params) - return [] if params.blank? + Gitlab::QuickActions::UsersExtractor + .new(current_user, project: project, group: group, target: quick_action_target, text: params) + .execute - # We are using the a simple User.by_username query here rather than a ReferenceExtractor - # because the needs here are much simpler: we only deal in usernames, and - # want to also handle bare usernames. The ReferenceExtractor also has - # different behaviour, and will return all group members for groups named - # using a user-style reference, which is not in scope here. - # - # nb: underscores may be passed in escaped to protect them from markdown rendering - args = params.split(/\s|,/).select(&:present?).uniq - ['and'] - args.map! { _1.gsub(/\\_/, '_') } - usernames = (args - ['me']).map { _1.delete_prefix('@') } - found = User.by_username(usernames).to_a.select { can?(:read_user, _1) } - found_names = found.map(&:username).map(&:downcase).to_set - missing = args.reject do |arg| - arg == 'me' || found_names.include?(arg.downcase.delete_prefix('@')) - end.map { "'#{_1}'" } + rescue Gitlab::QuickActions::UsersExtractor::Error => err + extract_users_failed(err) + end - failed_parse(format(_("Failed to find users for %{missing}"), missing: missing.to_sentence)) if missing.present? - - found + [current_user].select { args.include?('me') } + def extract_users_failed(err) + case err + when Gitlab::QuickActions::UsersExtractor::MissingError + failed_parse(format(_("Failed to find users for %{missing}"), missing: err.message)) + when Gitlab::QuickActions::UsersExtractor::TooManyRefsError + failed_parse(format(_('Too many references. Quick actions are limited to at most %{max_count} user references'), + max_count: err.limit)) + when Gitlab::QuickActions::UsersExtractor::TooManyFoundError + failed_parse(format(_("Too many users found. Quick actions are limited to at most %{max_count} users"), + max_count: err.limit)) + else + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(err) + failed_parse(_('Something went wrong')) + end end def find_milestones(project, params = {}) diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index 316e6367aa7..eed03ba22fe 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -108,7 +108,7 @@ module ResourceAccessTokens end def create_membership(resource, user, access_level) - resource.add_user(user, access_level, expires_at: params[:expires_at]) + resource.add_member(user, access_level, expires_at: params[:expires_at]) end def log_event(token) diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index b7cf7b7468f..59d4c81358d 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -17,7 +17,6 @@ = dispensable_render "shared/service_ping_consent" = dispensable_render_if_exists "layouts/header/ee_subscribable_banner" = dispensable_render_if_exists "layouts/header/seat_count_alert" - = dispensable_render_if_exists "shared/namespace_storage_limit_alert" = dispensable_render_if_exists "shared/namespace_user_cap_reached_alert" = dispensable_render_if_exists "shared/new_user_signups_cap_reached_alert" = yield :page_level_alert diff --git a/app/views/layouts/group.html.haml b/app/views/layouts/group.html.haml index 940724e0e4a..1c2ab8cf008 100644 --- a/app/views/layouts/group.html.haml +++ b/app/views/layouts/group.html.haml @@ -3,11 +3,11 @@ - header_title group_title(@group) unless header_title - nav "group" - display_subscription_banner! -- display_namespace_storage_limit_alert! - @left_sidebar = true - content_for :flash_message do = render "layouts/header/storage_enforcement_banner", namespace: @group + = dispensable_render_if_exists "shared/namespace_storage_limit_alert" - content_for :page_specific_javascripts do - if current_user diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml index 7e69cf1d685..86b4c4eabe3 100644 --- a/app/views/layouts/project.html.haml +++ b/app/views/layouts/project.html.haml @@ -4,12 +4,12 @@ - nav "project" - page_itemtype 'http://schema.org/SoftwareSourceCode' - display_subscription_banner! -- display_namespace_storage_limit_alert! - @left_sidebar = true - @content_class = [@content_class, project_classes(@project)].compact.join(" ") - content_for :flash_message do = render "layouts/header/storage_enforcement_banner", namespace: @project.namespace + = dispensable_render_if_exists "shared/namespace_storage_limit_alert" - content_for :project_javascripts do - project = @target_project || @project diff --git a/app/views/projects/mirrors/_ssh_host_keys.html.haml b/app/views/projects/mirrors/_ssh_host_keys.html.haml index e3fe098c807..d367f383e5a 100644 --- a/app/views/projects/mirrors/_ssh_host_keys.html.haml +++ b/app/views/projects/mirrors/_ssh_host_keys.html.haml @@ -11,7 +11,7 @@ = _('Fingerprints') .fingerprints-list.js-fingerprints-list{ data: { qa_selector: 'fingerprints_list' } } - mirror.ssh_known_hosts_fingerprints.each do |fp| - %code= fp.fingerprint + %code= fp.fingerprint_sha256 || fp.fingerprint - if verified_at .form-text.text-muted.js-fingerprint-verification = sprite_icon('check', css_class: 'gl-text-green-500') diff --git a/config/application.rb b/config/application.rb index 6b4a55d6d05..61f38787c1d 100644 --- a/config/application.rb +++ b/config/application.rb @@ -406,6 +406,16 @@ module Gitlab end end + # Cross-origin requests must be enabled to fetch the self-managed application oauth application ID + # for the GitLab for Jira app. + allow do + origins '*' + resource '/-/jira_connect/oauth_application_id', + headers: :any, + methods: %i(get options), + credentials: false + end + # These are routes from doorkeeper-openid_connect: # https://github.com/doorkeeper-gem/doorkeeper-openid_connect#routes allow do diff --git a/config/feature_flags/development/approval_rules_pagination.yml b/config/feature_flags/development/approval_rules_pagination.yml new file mode 100644 index 00000000000..494109e3f5a --- /dev/null +++ b/config/feature_flags/development/approval_rules_pagination.yml @@ -0,0 +1,8 @@ +--- +name: approval_rules_pagination +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91702 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/366823 +milestone: '15.2' +type: development +group: group::source code +default_enabled: false diff --git a/config/feature_flags/development/externally_stored_diffs_caching_export.yml b/config/feature_flags/development/externally_stored_diffs_caching_export.yml new file mode 100644 index 00000000000..395ce5c7cb9 --- /dev/null +++ b/config/feature_flags/development/externally_stored_diffs_caching_export.yml @@ -0,0 +1,8 @@ +--- +name: externally_stored_diffs_caching_export +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90159 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/366255 +milestone: '15.2' +type: development +group: group::import +default_enabled: false diff --git a/config/feature_flags/development/refactor_mr_widgets_extensions.yml b/config/feature_flags/development/refactor_mr_widgets_extensions.yml index 5b6ea22aafe..3f71e786f99 100644 --- a/config/feature_flags/development/refactor_mr_widgets_extensions.yml +++ b/config/feature_flags/development/refactor_mr_widgets_extensions.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/341759 milestone: '14.4' type: development group: group::code review -default_enabled: false +default_enabled: true diff --git a/config/routes.rb b/config/routes.rb index 44e89f0d387..652b50e3928 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -55,6 +55,8 @@ Rails.application.routes.draw do match '/oauth/token' => 'oauth/tokens#create', via: :options match '/oauth/revoke' => 'oauth/tokens#revoke', via: :options + match '/-/jira_connect/oauth_application_id' => 'jira_connect/oauth_application_ids#show', via: :options + # Sign up scope path: '/users/sign_up', module: :registrations, as: :users_sign_up do resource :welcome, only: [:show, :update], controller: 'welcome' do diff --git a/db/fixtures/development/06_teams.rb b/db/fixtures/development/06_teams.rb index 7aaaa48d6d4..71fe0df70f4 100644 --- a/db/fixtures/development/06_teams.rb +++ b/db/fixtures/development/06_teams.rb @@ -4,7 +4,7 @@ Sidekiq::Testing.inline! do Gitlab::Seeder.quiet do Group.not_mass_generated.each do |group| User.not_mass_generated.sample(4).each do |user| - if group.add_user(user, Gitlab::Access.values.sample).persisted? + if group.add_member(user, Gitlab::Access.values.sample).persisted? print '.' else print 'F' diff --git a/doc/development/fips_compliance.md b/doc/development/fips_compliance.md index 5b6f6ba0d98..aea23620d22 100644 --- a/doc/development/fips_compliance.md +++ b/doc/development/fips_compliance.md @@ -396,6 +396,16 @@ gitlab: tag: v15.1.0-fips ``` +## FIPS Performance Benchmarking + +The Quality Engineering Enablement team assists these efforts by checking if FIPS-enabled environments perform well compared to non-FIPS environments. + +Testing shows an impact in some places, such as Gitaly SSL, but it's not large enough to impact customers. + +You can find more information on FIPS performance benchmarking in the following issue: + +- [Benchmark performance of FIPS reference architecture](https://gitlab.com/gitlab-org/gitlab/-/issues/364051#note_1010450415) + ## Verify FIPS The following sections describe ways you can verify if FIPS is enabled. diff --git a/doc/user/group/settings/group_access_tokens.md b/doc/user/group/settings/group_access_tokens.md index 5e426888ad9..9e8fc120731 100644 --- a/doc/user/group/settings/group_access_tokens.md +++ b/doc/user/group/settings/group_access_tokens.md @@ -78,7 +78,7 @@ or API. However, administrators can use a workaround: bot.confirm # Add the bot to the group with the required role. - group.add_user(bot, :maintainer) + group.add_member(bot, :maintainer) # Give the bot a personal access token. token = bot.personal_access_tokens.create(scopes:[:api, :write_repository], name: 'group_token') diff --git a/lib/gitlab/ci/variables/builder.rb b/lib/gitlab/ci/variables/builder.rb index 158559509b8..95dff83506d 100644 --- a/lib/gitlab/ci/variables/builder.rb +++ b/lib/gitlab/ci/variables/builder.rb @@ -26,7 +26,6 @@ module Gitlab variables.concat(secret_instance_variables) variables.concat(secret_group_variables(environment: environment)) variables.concat(secret_project_variables(environment: environment)) - variables.concat(job.trigger_request.user_variables) if job.trigger_request variables.concat(pipeline.variables) variables.concat(pipeline_schedule_variables) end diff --git a/lib/gitlab/database_importers/instance_administrators/create_group.rb b/lib/gitlab/database_importers/instance_administrators/create_group.rb index 79244120776..bb489ced3d2 100644 --- a/lib/gitlab/database_importers/instance_administrators/create_group.rb +++ b/lib/gitlab/database_importers/instance_administrators/create_group.rb @@ -77,7 +77,7 @@ module Gitlab def add_group_members(result) group = result[:group] - members = group.add_users(members_to_add(group), Gitlab::Access::MAINTAINER) + members = group.add_members(members_to_add(group), Gitlab::Access::MAINTAINER) errors = members.flat_map { |member| member.errors.full_messages } if errors.any? @@ -112,7 +112,7 @@ module Gitlab def members_to_add(group) # Exclude admins who are already members of group because - # `group.add_users(users)` returns an error if the users parameter contains + # `group.add_members(users)` returns an error if the users parameter contains # users who are already members of the group. instance_admins - group.members.collect(&:user) end diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb index ebabf537ce5..59396c6bad2 100644 --- a/lib/gitlab/import_export/json/streaming_serializer.rb +++ b/lib/gitlab/import_export/json/streaming_serializer.rb @@ -70,7 +70,11 @@ module Gitlab batch = batch.preload(key_preloads) if key_preloads batch.each do |record| + before_read_callback(record) + items << Raw.new(record.to_json(options)) + + after_read_callback(record) end end end @@ -168,6 +172,20 @@ module Gitlab def read_from_replica_if_available(&block) ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block) end + + def before_read_callback(record) + remove_cached_external_diff(record) + end + + def after_read_callback(record) + remove_cached_external_diff(record) + end + + def remove_cached_external_diff(record) + return unless record.is_a?(MergeRequest) + + record.merge_request_diff&.remove_cached_external_diff + end end end end diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index 50ff6146174..22c98981f64 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -977,7 +977,7 @@ methods: statuses: - :type merge_request_diff_files: - - :utf8_diff + - :diff_export merge_requests: - :diff_head_sha - :source_branch_sha diff --git a/lib/gitlab/import_export/project/relation_factory.rb b/lib/gitlab/import_export/project/relation_factory.rb index 8110720fb46..32e653fb6e4 100644 --- a/lib/gitlab/import_export/project/relation_factory.rb +++ b/lib/gitlab/import_export/project/relation_factory.rb @@ -133,7 +133,7 @@ module Gitlab end def setup_diff - diff = @relation_hash.delete('utf8_diff') + diff = @relation_hash.delete('diff_export') || @relation_hash.delete('utf8_diff') parsed_relation_hash['diff'] = diff end diff --git a/lib/gitlab/quick_actions/users_extractor.rb b/lib/gitlab/quick_actions/users_extractor.rb new file mode 100644 index 00000000000..06e04c74312 --- /dev/null +++ b/lib/gitlab/quick_actions/users_extractor.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module Gitlab + module QuickActions + class UsersExtractor + MAX_QUICK_ACTION_USERS = 100 + + Error = Class.new(ArgumentError) + TooManyError = Class.new(Error) do + def limit + MAX_QUICK_ACTION_USERS + end + end + + MissingError = Class.new(Error) + TooManyFoundError = Class.new(TooManyError) + TooManyRefsError = Class.new(TooManyError) + + attr_reader :text, :current_user, :project, :group, :target + + def initialize(current_user, project:, group:, target:, text:) + @current_user = current_user + @project = project + @group = group + @target = target + @text = text + end + + def execute + return [] unless text.present? + + users = collect_users + + check_users!(users) + + users + end + + private + + def collect_users + users = [] + users << current_user if me? + users += find_referenced_users if references.any? + + users + end + + def check_users!(users) + raise TooManyFoundError if users.size > MAX_QUICK_ACTION_USERS + + found = found_names(users) + missing = references.filter_map do + "'#{_1}'" unless found.include?(_1.downcase.delete_prefix('@')) + end + + raise MissingError, missing.to_sentence if missing.present? + end + + def found_names(users) + users.map(&:username).map(&:downcase).to_set + end + + def find_referenced_users + raise TooManyRefsError if references.size > MAX_QUICK_ACTION_USERS + + User.by_username(usernames).limit(MAX_QUICK_ACTION_USERS) + end + + def usernames + references.map { _1.delete_prefix('@') } + end + + def references + @references ||= begin + refs = args - ['me'] + # nb: underscores may be passed in escaped to protect them from markdown rendering + refs.map! { _1.gsub(/\\_/, '_') } + refs + end + end + + def args + @args ||= text.split(/\s|,/).map(&:strip).select(&:present?).uniq - ['and'] + end + + def me? + args&.include?('me') + end + end + end +end + +Gitlab::QuickActions::UsersExtractor.prepend_mod_with('Gitlab::QuickActions::UsersExtractor') diff --git a/lib/tasks/gitlab/bulk_add_permission.rake b/lib/tasks/gitlab/bulk_add_permission.rake index df0c6a260a2..a903c743ea2 100644 --- a/lib/tasks/gitlab/bulk_add_permission.rake +++ b/lib/tasks/gitlab/bulk_add_permission.rake @@ -9,10 +9,10 @@ namespace :gitlab do project_ids = Project.pluck(:id) puts "Importing #{user_ids.size} users into #{project_ids.size} projects" - ProjectMember.add_users_to_projects(project_ids, user_ids, ProjectMember::DEVELOPER) + ProjectMember.add_members_to_projects(project_ids, user_ids, ProjectMember::DEVELOPER) puts "Importing #{admin_ids.size} admins into #{project_ids.size} projects" - ProjectMember.add_users_to_projects(project_ids, admin_ids, ProjectMember::MAINTAINER) + ProjectMember.add_members_to_projects(project_ids, admin_ids, ProjectMember::MAINTAINER) end desc "GitLab | Import | Add a specific user to all projects (as a developer)" @@ -20,7 +20,7 @@ namespace :gitlab do user = User.find_by(email: args.email) project_ids = Project.pluck(:id) puts "Importing #{user.email} users into #{project_ids.size} projects" - ProjectMember.add_users_to_projects(project_ids, Array.wrap(user.id), ProjectMember::DEVELOPER) + ProjectMember.add_members_to_projects(project_ids, Array.wrap(user.id), ProjectMember::DEVELOPER) end desc "GitLab | Import | Add all users to all groups (admin users are added as owners)" @@ -32,8 +32,8 @@ namespace :gitlab do puts "Importing #{user_ids.size} users into #{groups.size} groups" puts "Importing #{admin_ids.size} admins into #{groups.size} groups" groups.each do |group| - group.add_users(user_ids, GroupMember::DEVELOPER) - group.add_users(admin_ids, GroupMember::OWNER) + group.add_members(user_ids, GroupMember::DEVELOPER) + group.add_members(admin_ids, GroupMember::OWNER) end end @@ -43,7 +43,7 @@ namespace :gitlab do groups = Group.all puts "Importing #{user.email} users into #{groups.size} groups" groups.each do |group| - group.add_users(Array.wrap(user.id), GroupMember::DEVELOPER) + group.add_members(Array.wrap(user.id), GroupMember::DEVELOPER) end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cb179eaa7dc..91df755949a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -40498,6 +40498,12 @@ msgstr "" msgid "Too many projects enabled. Manage them through the console or the API." msgstr "" +msgid "Too many references. Quick actions are limited to at most %{max_count} user references" +msgstr "" + +msgid "Too many users found. Quick actions are limited to at most %{max_count} users" +msgstr "" + msgid "TopNav|Go back" msgstr "" diff --git a/qa/qa.rb b/qa/qa.rb index 8d358aa2efe..b175275903a 100644 --- a/qa/qa.rb +++ b/qa/qa.rb @@ -21,6 +21,10 @@ module QA root = "#{__dir__}/qa" loader = Zeitwerk::Loader.new + + # require jh/qa/qa.rb first, to load JH module make prepend module works + require '../jh/qa/qa' if GitlabEdition.jh? + loader.push_dir(root, namespace: QA) loader.ignore("#{root}/specs/features") diff --git a/qa/qa/scenario/template.rb b/qa/qa/scenario/template.rb index 99906ca44d9..56ab4e14352 100644 --- a/qa/qa/scenario/template.rb +++ b/qa/qa/scenario/template.rb @@ -86,3 +86,5 @@ module QA end end end + +QA::Scenario::Template.prepend_mod_with('Scenario::Template', namespace: QA) diff --git a/spec/controllers/groups/variables_controller_spec.rb b/spec/controllers/groups/variables_controller_spec.rb index 8c0aa83b9c4..6dbe75bb1df 100644 --- a/spec/controllers/groups/variables_controller_spec.rb +++ b/spec/controllers/groups/variables_controller_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Groups::VariablesController do before do sign_in(user) - group.add_user(user, access_level) + group.add_member(user, access_level) end describe 'GET #show' do diff --git a/spec/controllers/import/available_namespaces_controller_spec.rb b/spec/controllers/import/available_namespaces_controller_spec.rb index 0f98d649338..26ea1d92189 100644 --- a/spec/controllers/import/available_namespaces_controller_spec.rb +++ b/spec/controllers/import/available_namespaces_controller_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Import::AvailableNamespacesController do it "does not include group with access level #{params[:role]} in list" do group = create(:group, project_creation_level: group_project_creation_level) - group.add_user(user, role) + group.add_member(user, role) get :index expect(response).to have_gitlab_http_status(:ok) @@ -52,7 +52,7 @@ RSpec.describe Import::AvailableNamespacesController do it "does not include group with access level #{params[:role]} in list" do group = create(:group, project_creation_level: group_project_creation_level) - group.add_user(user, role) + group.add_member(user, role) get :index expect(response).to have_gitlab_http_status(:ok) @@ -81,7 +81,7 @@ RSpec.describe Import::AvailableNamespacesController do it "#{params[:is_visible] ? 'includes' : 'does not include'} group with access level #{params[:role]} in list" do group = create(:group, project_creation_level: project_creation_level) - group.add_user(user, :developer) + group.add_member(user, :developer) get :index diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 1305693372c..badac688229 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1818,7 +1818,7 @@ RSpec.describe Projects::IssuesController do context 'user is allowed access' do before do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) end it 'displays all available notes' do diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index 686effd799e..d33bc215cfc 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -211,7 +211,7 @@ RSpec.describe Projects::MirrorsController do context 'data in the cache' do let(:ssh_key) { 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAfuCHKVTjquxvt6CM6tdG4SLp1Btn/nOeHHE5UOzRdf' } - let(:ssh_fp) { { type: 'ed25519', bits: 256, fingerprint: '2e:65:6a:c8:cf:bf:b2:8b:9a:bd:6d:9f:11:5c:12:16', index: 0 } } + let(:ssh_fp) { { type: 'ed25519', bits: 256, fingerprint: '2e:65:6a:c8:cf:bf:b2:8b:9a:bd:6d:9f:11:5c:12:16', fingerprint_sha256: 'SHA256:eUXGGm1YGsMAS7vkcx6JOJdOGHPem5gQp4taiCfCLB8', index: 0 } } it 'returns the data with a 200 response' do stub_reactive_cache(cache, known_hosts: ssh_key) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index d9d26a68785..34477a7bb68 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -409,7 +409,7 @@ RSpec.describe ProjectsController do before do project.update!(visibility: project_visibility.to_s) - project.team.add_user(user, :guest) if user_type == :member + project.team.add_member(user, :guest) if user_type == :member sign_in(user) unless user_type == :anonymous end diff --git a/spec/features/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index f903d7c2db2..040c6a65b7c 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -236,7 +236,7 @@ RSpec.describe 'Admin Groups' do it 'renders relative time' do expire_time = Time.current + 2.days current_user.update!(time_display_relative: true) - group.add_user(user, Gitlab::Access::REPORTER, expires_at: expire_time) + group.add_member(user, Gitlab::Access::REPORTER, expires_at: expire_time) visit admin_group_path(group) @@ -246,7 +246,7 @@ RSpec.describe 'Admin Groups' do it 'renders absolute time' do expire_time = Time.current.tomorrow.middle_of_day current_user.update!(time_display_relative: false) - group.add_user(user, Gitlab::Access::REPORTER, expires_at: expire_time) + group.add_member(user, Gitlab::Access::REPORTER, expires_at: expire_time) visit admin_group_path(group) @@ -257,7 +257,7 @@ RSpec.describe 'Admin Groups' do describe 'add admin himself to a group' do before do - group.add_user(:user, Gitlab::Access::OWNER) + group.add_member(:user, Gitlab::Access::OWNER) end it 'adds admin a to a group as developer', :js do @@ -274,7 +274,7 @@ RSpec.describe 'Admin Groups' do describe 'admin removes themself from a group', :js do it 'removes admin from the group' do - group.add_user(current_user, Gitlab::Access::DEVELOPER) + group.add_member(current_user, Gitlab::Access::DEVELOPER) visit group_group_members_path(group) diff --git a/spec/features/admin/admin_projects_spec.rb b/spec/features/admin/admin_projects_spec.rb index 866368af40a..6b147b01991 100644 --- a/spec/features/admin/admin_projects_spec.rb +++ b/spec/features/admin/admin_projects_spec.rb @@ -20,7 +20,7 @@ RSpec.describe "Admin::Projects" do it 'renders relative time' do expire_time = Time.current + 2.days current_user.update!(time_display_relative: true) - project.add_user(user, Gitlab::Access::REPORTER, expires_at: expire_time) + project.add_member(user, Gitlab::Access::REPORTER, expires_at: expire_time) visit admin_project_path(project) @@ -30,7 +30,7 @@ RSpec.describe "Admin::Projects" do it 'renders absolute time' do expire_time = Time.current.tomorrow.middle_of_day current_user.update!(time_display_relative: false) - project.add_user(user, Gitlab::Access::REPORTER, expires_at: expire_time) + project.add_member(user, Gitlab::Access::REPORTER, expires_at: expire_time) visit admin_project_path(project) diff --git a/spec/features/file_uploads/multipart_invalid_uploads_spec.rb b/spec/features/file_uploads/multipart_invalid_uploads_spec.rb index 91c8e100e6a..cff8b4e61a5 100644 --- a/spec/features/file_uploads/multipart_invalid_uploads_spec.rb +++ b/spec/features/file_uploads/multipart_invalid_uploads_spec.rb @@ -44,7 +44,7 @@ RSpec.describe 'Invalid uploads that must be rejected', :api, :js do # These keys are rejected directly by rack itself. # The request will not be received by multipart.rb (can't use the 'handling file uploads' shared example) - it_behaves_like 'rejecting invalid keys', key_name: 'x' * 11000, message: 'Puma caught this error: exceeded available parameter key space (RangeError)' + it_behaves_like 'rejecting invalid keys', key_name: 'x' * 11000, message: 'Puma caught this error: exceeded available parameter key space (Rack::QueryParser::ParamsTooDeepError)' it_behaves_like 'rejecting invalid keys', key_name: 'package[]test', status: 400, message: 'Bad Request' it_behaves_like 'handling file uploads', 'by rejecting uploads with an invalid key' diff --git a/spec/features/issuables/issuable_list_spec.rb b/spec/features/issuables/issuable_list_spec.rb index 0fa2d238b0a..a1e80586c05 100644 --- a/spec/features/issuables/issuable_list_spec.rb +++ b/spec/features/issuables/issuable_list_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'issuable list', :js do issuable_types = [:issue, :merge_request] before do - project.add_user(user, :developer) + project.add_member(user, :developer) sign_in(user) issuable_types.each { |type| create_issuables(type) } end diff --git a/spec/features/issues/filtered_search/visual_tokens_spec.rb b/spec/features/issues/filtered_search/visual_tokens_spec.rb index 7a367723609..c44181a60e4 100644 --- a/spec/features/issues/filtered_search/visual_tokens_spec.rb +++ b/spec/features/issues/filtered_search/visual_tokens_spec.rb @@ -15,8 +15,8 @@ RSpec.describe 'Visual tokens', :js do let_it_be(:issue) { create(:issue, project: project) } before do - project.add_user(user, :maintainer) - project.add_user(user_rock, :maintainer) + project.add_member(user, :maintainer) + project.add_member(user_rock, :maintainer) sign_in(user) visit project_issues_path(project) diff --git a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb index ae1bce7ea4c..5ba09703852 100644 --- a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb +++ b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb @@ -20,7 +20,7 @@ RSpec.describe 'User creates branch and merge request on issue page', :js do context 'when signed in' do before do - project.add_user(user, membership_level) + project.add_member(user, membership_level) sign_in(user) end diff --git a/spec/features/merge_request/user_sees_deployment_widget_spec.rb b/spec/features/merge_request/user_sees_deployment_widget_spec.rb index 81034caaee2..e045f11c0d8 100644 --- a/spec/features/merge_request/user_sees_deployment_widget_spec.rb +++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb @@ -20,7 +20,7 @@ RSpec.describe 'Merge request > User sees deployment widget', :js do before do merge_request.update!(merge_commit_sha: sha) - project.add_user(user, role) + project.add_member(user, role) sign_in(user) end diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index bd0874316ac..c92e8bc2954 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -32,7 +32,7 @@ RSpec.describe 'Projects > Members > Maintainer adds member with expiration date end it 'changes expiration date' do - project.team.add_users([new_member.id], :developer, expires_at: three_days_from_now) + project.team.add_members([new_member.id], :developer, expires_at: three_days_from_now) visit project_project_members_path(project) page.within find_member_row(new_member) do @@ -46,7 +46,7 @@ RSpec.describe 'Projects > Members > Maintainer adds member with expiration date end it 'clears expiration date' do - project.team.add_users([new_member.id], :developer, expires_at: five_days_from_now) + project.team.add_members([new_member.id], :developer, expires_at: five_days_from_now) visit project_project_members_path(project) page.within find_member_row(new_member) do diff --git a/spec/features/projects/show/user_interacts_with_auto_devops_banner_spec.rb b/spec/features/projects/show/user_interacts_with_auto_devops_banner_spec.rb index 59f1bc94226..262885e09b3 100644 --- a/spec/features/projects/show/user_interacts_with_auto_devops_banner_spec.rb +++ b/spec/features/projects/show/user_interacts_with_auto_devops_banner_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Project > Show > User interacts with auto devops implicitly enab let(:user) { create(:user) } before do - project.add_user(user, role) + project.add_member(user, role) sign_in(user) end diff --git a/spec/features/projects/show/user_sees_collaboration_links_spec.rb b/spec/features/projects/show/user_sees_collaboration_links_spec.rb index 552f068ecc7..fb2f0539558 100644 --- a/spec/features/projects/show/user_sees_collaboration_links_spec.rb +++ b/spec/features/projects/show/user_sees_collaboration_links_spec.rb @@ -90,7 +90,7 @@ RSpec.describe 'Projects > Show > Collaboration links', :js do with_them do before do project.project_feature.update!({ merge_requests_access_level: merge_requests_access_level }) - project.add_user(user, user_level) + project.add_member(user, user_level) visit project_path(project) end diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index efb7c98d63a..3ba3650b608 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -579,9 +579,9 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do context 'group setting' do before do group1 = create :group, name: 'Group 1', require_two_factor_authentication: true - group1.add_user(user, GroupMember::DEVELOPER) + group1.add_member(user, GroupMember::DEVELOPER) group2 = create :group, name: 'Group 2', require_two_factor_authentication: true - group2.add_user(user, GroupMember::DEVELOPER) + group2.add_member(user, GroupMember::DEVELOPER) end context 'with grace period defined' do diff --git a/spec/finders/autocomplete/users_finder_spec.rb b/spec/finders/autocomplete/users_finder_spec.rb index 9b3421d1b4f..de031041e18 100644 --- a/spec/finders/autocomplete/users_finder_spec.rb +++ b/spec/finders/autocomplete/users_finder_spec.rb @@ -62,7 +62,7 @@ RSpec.describe Autocomplete::UsersFinder do let_it_be(:group) { create(:group, :public) } before_all do - group.add_users([user1], GroupMember::DEVELOPER) + group.add_members([user1], GroupMember::DEVELOPER) end it { is_expected.to match_array([user1]) } diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb index 058db735708..feb1b11d159 100644 --- a/spec/finders/joined_groups_finder_spec.rb +++ b/spec/finders/joined_groups_finder_spec.rb @@ -45,7 +45,7 @@ RSpec.describe JoinedGroupsFinder do context 'if profile visitor is in one of the private group projects' do before do project = create(:project, :private, group: private_group, name: 'B', path: 'B') - project.add_user(profile_visitor, Gitlab::Access::DEVELOPER) + project.add_member(profile_visitor, Gitlab::Access::DEVELOPER) end it 'shows group' do diff --git a/spec/finders/packages/conan/package_finder_spec.rb b/spec/finders/packages/conan/package_finder_spec.rb index 6848786818b..f25a62225a8 100644 --- a/spec/finders/packages/conan/package_finder_spec.rb +++ b/spec/finders/packages/conan/package_finder_spec.rb @@ -45,7 +45,7 @@ RSpec.describe ::Packages::Conan::PackageFinder do before do project.update_column(:visibility_level, Gitlab::VisibilityLevel.string_options[visibility.to_s]) - project.add_user(user, role) unless role == :anonymous + project.add_member(user, role) unless role == :anonymous end it { is_expected.to eq(expected_packages) } diff --git a/spec/finders/packages/group_packages_finder_spec.rb b/spec/finders/packages/group_packages_finder_spec.rb index 954db6481cd..90a8cd3c57f 100644 --- a/spec/finders/packages/group_packages_finder_spec.rb +++ b/spec/finders/packages/group_packages_finder_spec.rb @@ -98,8 +98,8 @@ RSpec.describe Packages::GroupPackagesFinder do ) unless role == :anonymous - project.add_user(user, role) - subproject.add_user(user, role) + project.add_member(user, role) + subproject.add_member(user, role) end end diff --git a/spec/frontend/ide/components/new_dropdown/modal_spec.js b/spec/frontend/ide/components/new_dropdown/modal_spec.js index e8635444801..8c72a37ec84 100644 --- a/spec/frontend/ide/components/new_dropdown/modal_spec.js +++ b/spec/frontend/ide/components/new_dropdown/modal_spec.js @@ -1,209 +1,373 @@ -import Vue, { nextTick } from 'vue'; -import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; +import { GlButton, GlModal } from '@gitlab/ui'; +import { nextTick } from 'vue'; import createFlash from '~/flash'; -import modal from '~/ide/components/new_dropdown/modal.vue'; +import Modal from '~/ide/components/new_dropdown/modal.vue'; import { createStore } from '~/ide/stores'; +import { stubComponent } from 'helpers/stub_component'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { createEntriesFromPaths } from '../../helpers'; jest.mock('~/flash'); +const NEW_NAME = 'babar'; + describe('new file modal component', () => { - const Component = Vue.extend(modal); - let vm; + const showModal = jest.fn(); + const toggleModal = jest.fn(); + + let store; + let wrapper; + + const findGlModal = () => wrapper.findComponent(GlModal); + const findInput = () => wrapper.findByTestId('file-name-field'); + const findTemplateButtons = () => wrapper.findAllComponents(GlButton); + const findTemplateButtonsModel = () => + findTemplateButtons().wrappers.map((x) => ({ + text: x.text(), + variant: x.props('variant'), + category: x.props('category'), + })); + + const open = (type, path) => { + // TODO: This component can not be passed props + // We have to interact with the open() method? + wrapper.vm.open(type, path); + }; + const triggerSubmit = () => { + findGlModal().vm.$emit('primary'); + }; + const triggerCancel = () => { + findGlModal().vm.$emit('cancel'); + }; + + const mountComponent = () => { + const GlModalStub = stubComponent(GlModal); + jest.spyOn(GlModalStub.methods, 'show').mockImplementation(showModal); + jest.spyOn(GlModalStub.methods, 'toggle').mockImplementation(toggleModal); + + wrapper = shallowMountExtended(Modal, { + store, + stubs: { + GlModal: GlModalStub, + }, + // We need to attach to document for "focus" to work + attachTo: document.body, + }); + }; + + beforeEach(() => { + store = createStore(); + + Object.assign( + store.state.entries, + createEntriesFromPaths([ + 'README.md', + 'src', + 'src/deleted.js', + 'src/parent_dir', + 'src/parent_dir/foo.js', + ]), + ); + Object.assign(store.state.entries['src/deleted.js'], { deleted: true }); + + jest.spyOn(store, 'dispatch').mockImplementation(); + }); afterEach(() => { - vm.$destroy(); + store = null; + wrapper.destroy(); + document.body.innerHTML = ''; + }); + + describe('default', () => { + beforeEach(async () => { + mountComponent(); + + // Not necessarily needed, but used to ensure that nothing extra is happening after the tick + await nextTick(); + }); + + it('renders modal', () => { + expect(findGlModal().props()).toMatchObject({ + actionCancel: { + attributes: [{ variant: 'default' }], + text: 'Cancel', + }, + actionPrimary: { + attributes: [{ variant: 'confirm' }], + text: 'Create file', + }, + actionSecondary: null, + size: 'lg', + modalId: 'ide-new-entry', + title: 'Create new file', + }); + }); + + it('renders name label', () => { + expect(wrapper.find('label').text()).toBe('Name'); + }); + + it('renders template buttons', () => { + const actual = findTemplateButtonsModel(); + + expect(actual.length).toBeGreaterThan(0); + expect(actual).toEqual( + store.getters['fileTemplates/templateTypes'].map((template) => ({ + category: 'secondary', + text: template.name, + variant: 'dashed', + })), + ); + }); + + // These negative ".not.toHaveBeenCalled" assertions complement the positive "toHaveBeenCalled" + // assertions that show up later in this spec. Without these, we're not guaranteed the "act" + // actually caused the change in behavior. + it('does not dispatch actions by default', () => { + expect(store.dispatch).not.toHaveBeenCalled(); + }); + + it('does not trigger modal by default', () => { + expect(showModal).not.toHaveBeenCalled(); + expect(toggleModal).not.toHaveBeenCalled(); + }); + + it('does not focus input by default', () => { + expect(document.activeElement).toBe(document.body); + }); }); describe.each` - entryType | modalTitle | btnTitle | showsFileTemplates - ${'tree'} | ${'Create new directory'} | ${'Create directory'} | ${false} - ${'blob'} | ${'Create new file'} | ${'Create file'} | ${true} - `('$entryType', ({ entryType, modalTitle, btnTitle, showsFileTemplates }) => { - beforeEach(async () => { - const store = createStore(); + entryType | path | modalTitle | btnTitle | showsFileTemplates | inputValue | inputPlaceholder + ${'tree'} | ${''} | ${'Create new directory'} | ${'Create directory'} | ${false} | ${''} | ${'dir/'} + ${'blob'} | ${''} | ${'Create new file'} | ${'Create file'} | ${true} | ${''} | ${'dir/file_name'} + ${'blob'} | ${'foo/bar'} | ${'Create new file'} | ${'Create file'} | ${true} | ${'foo/bar/'} | ${'dir/file_name'} + `( + 'when opened as $entryType with path "$path"', + ({ + entryType, + path, + modalTitle, + btnTitle, + showsFileTemplates, + inputValue, + inputPlaceholder, + }) => { + beforeEach(async () => { + mountComponent(); - vm = createComponentWithStore(Component, store).$mount(); - vm.open(entryType); - vm.name = 'testing'; + open(entryType, path); + + await nextTick(); + }); + + it('sets modal props', () => { + expect(findGlModal().props()).toMatchObject({ + title: modalTitle, + actionPrimary: { + attributes: [{ variant: 'confirm' }], + text: btnTitle, + }, + }); + }); + + it('sets input attributes', () => { + expect(findInput().element.value).toBe(inputValue); + expect(findInput().attributes('placeholder')).toBe(inputPlaceholder); + }); + + it(`shows file templates: ${showsFileTemplates}`, () => { + const actual = findTemplateButtonsModel().length > 0; + + expect(actual).toBe(showsFileTemplates); + }); + + it('shows modal', () => { + expect(showModal).toHaveBeenCalled(); + }); + + it('focus on input', () => { + expect(document.activeElement).toBe(findInput().element); + }); + + it('resets when canceled', async () => { + triggerCancel(); + + await nextTick(); + + // Resets input value + expect(findInput().element.value).toBe(''); + // Resets to blob mode + expect(findGlModal().props('title')).toBe('Create new file'); + }); + }, + ); + + describe.each` + modalType | name | expectedName + ${'blob'} | ${'foo/bar.js'} | ${'foo/bar.js'} + ${'blob'} | ${'foo /bar.js'} | ${'foo/bar.js'} + ${'tree'} | ${'foo/dir'} | ${'foo/dir'} + ${'tree'} | ${'foo /dir'} | ${'foo/dir'} + `('when submitting as $modalType with "$name"', ({ modalType, name, expectedName }) => { + beforeEach(async () => { + mountComponent(); + + open(modalType, ''); + await nextTick(); + + findInput().setValue(name); + triggerSubmit(); + }); + + it('triggers createTempEntry action', () => { + expect(store.dispatch).toHaveBeenCalledWith('createTempEntry', { + name: expectedName, + type: modalType, + }); + }); + }); + + describe('when creating from template type', () => { + beforeEach(async () => { + mountComponent(); + + open('blob', 'some_dir'); + + await nextTick(); + + // Set input, then trigger button + findInput().setValue('some_dir/foo.js'); + findTemplateButtons().at(1).vm.$emit('click'); + }); + + it('triggers createTempEntry action', () => { + const { name: expectedName } = store.getters['fileTemplates/templateTypes'][1]; + + expect(store.dispatch).toHaveBeenCalledWith('createTempEntry', { + name: `some_dir/${expectedName}`, + type: 'blob', + }); + }); + + it('toggles modal', () => { + expect(toggleModal).toHaveBeenCalled(); + }); + }); + + describe.each` + origPath | title | inputValue | inputSelectionStart + ${'src/parent_dir'} | ${'Rename folder'} | ${'src/parent_dir'} | ${'src/'.length} + ${'README.md'} | ${'Rename file'} | ${'README.md'} | ${0} + `('when renaming for $origPath', ({ origPath, title, inputValue, inputSelectionStart }) => { + beforeEach(async () => { + mountComponent(); + + open('rename', origPath); await nextTick(); }); - afterEach(() => { - vm.close(); - }); - - it(`sets modal title as ${entryType}`, () => { - expect(document.querySelector('.modal-title').textContent.trim()).toBe(modalTitle); - }); - - it(`sets button label as ${entryType}`, () => { - expect(document.querySelector('.btn-confirm').textContent.trim()).toBe(btnTitle); - }); - - it(`sets form label as ${entryType}`, () => { - expect(document.querySelector('.label-bold').textContent.trim()).toBe('Name'); - }); - - it(`shows file templates: ${showsFileTemplates}`, () => { - const templateFilesEl = document.querySelector('.file-templates'); - expect(Boolean(templateFilesEl)).toBe(showsFileTemplates); - }); - }); - - describe('rename entry', () => { - beforeEach(() => { - const store = createStore(); - store.state.entries = { - 'test-path': { - name: 'test', - type: 'blob', - path: 'test-path', + it('sets modal props for renaming', () => { + expect(findGlModal().props()).toMatchObject({ + title, + actionPrimary: { + attributes: [{ variant: 'confirm' }], + text: title, }, - }; - - vm = createComponentWithStore(Component, store).$mount(); - }); - - it.each` - entryType | modalTitle | btnTitle - ${'tree'} | ${'Rename folder'} | ${'Rename folder'} - ${'blob'} | ${'Rename file'} | ${'Rename file'} - `( - 'renders title and button for renaming $entryType', - async ({ entryType, modalTitle, btnTitle }) => { - vm.$store.state.entries['test-path'].type = entryType; - vm.open('rename', 'test-path'); - - await nextTick(); - expect(document.querySelector('.modal-title').textContent.trim()).toBe(modalTitle); - expect(document.querySelector('.btn-confirm').textContent.trim()).toBe(btnTitle); - }, - ); - - describe('entryName', () => { - it('returns entries name', () => { - vm.open('rename', 'test-path'); - - expect(vm.entryName).toBe('test-path'); - }); - - it('does not reset entryName to its old value if empty', () => { - vm.entryName = 'hello'; - vm.entryName = ''; - - expect(vm.entryName).toBe(''); }); }); - describe('open', () => { - it('sets entryName to path provided if modalType is rename', () => { - vm.open('rename', 'test-path'); - - expect(vm.entryName).toBe('test-path'); - }); - - it("appends '/' to the path if modalType isn't rename", () => { - vm.open('blob', 'test-path'); - - expect(vm.entryName).toBe('test-path/'); - }); - - it('leaves entryName blank if no path is provided', () => { - vm.open('blob'); - - expect(vm.entryName).toBe(''); - }); - }); - }); - - describe('createFromTemplate', () => { - let store; - - beforeEach(() => { - store = createStore(); - store.state.entries = { - 'test-path/test': { - name: 'test', - deleted: false, - }, - }; - - vm = createComponentWithStore(Component, store).$mount(); - vm.open('blob'); - - jest.spyOn(vm, 'createTempEntry').mockImplementation(); + it('sets input value', () => { + expect(findInput().element.value).toBe(inputValue); }); - it.each` - entryName | newFilePath - ${''} | ${'.gitignore'} - ${'README.md'} | ${'.gitignore'} - ${'test-path/test/'} | ${'test-path/test/.gitignore'} - ${'test-path/test'} | ${'test-path/.gitignore'} - ${'test-path/test/abc.md'} | ${'test-path/test/.gitignore'} - `( - 'creates a new file with the given template name in appropriate directory for path: $path', - ({ entryName, newFilePath }) => { - vm.entryName = entryName; + it(`does not show file templates`, () => { + expect(findTemplateButtonsModel()).toHaveLength(0); + }); - vm.createFromTemplate({ name: '.gitignore' }); + it('shows modal when renaming', () => { + expect(showModal).toHaveBeenCalled(); + }); - expect(vm.createTempEntry).toHaveBeenCalledWith({ - name: newFilePath, - type: 'blob', + it('focus on input when renaming', () => { + expect(document.activeElement).toBe(findInput().element); + }); + + it('selects name part of the input', () => { + expect(findInput().element.selectionStart).toBe(inputSelectionStart); + expect(findInput().element.selectionEnd).toBe(origPath.length); + }); + + describe('when renames is submitted successfully', () => { + beforeEach(() => { + findInput().setValue(NEW_NAME); + triggerSubmit(); + }); + + it('dispatches renameEntry event', () => { + expect(store.dispatch).toHaveBeenCalledWith('renameEntry', { + path: origPath, + parentPath: '', + name: NEW_NAME, }); - }, - ); + }); + + it('does not trigger flash', () => { + expect(createFlash).not.toHaveBeenCalled(); + }); + }); }); - describe('submitForm', () => { - let store; + describe('when renaming and file already exists', () => { + beforeEach(async () => { + mountComponent(); - beforeEach(() => { - store = createStore(); - store.state.entries = { - 'test-path/test': { - name: 'test', - deleted: false, - }, - }; + open('rename', 'src/parent_dir'); - vm = createComponentWithStore(Component, store).$mount(); + await nextTick(); + + // Set to something that already exists! + findInput().setValue('src'); + triggerSubmit(); }); - it('throws an error when target entry exists', () => { - vm.open('rename', 'test-path/test'); - - expect(createFlash).not.toHaveBeenCalled(); - - vm.submitForm(); - + it('creates flash', () => { expect(createFlash).toHaveBeenCalledWith({ - message: 'The name "test-path/test" is already taken in this directory.', + message: 'The name "src" is already taken in this directory.', fadeTransition: false, addBodyClass: true, }); }); - it('does not throw error when target entry does not exist', () => { - jest.spyOn(vm, 'renameEntry').mockImplementation(); + it('does not dispatch event', () => { + expect(store.dispatch).not.toHaveBeenCalled(); + }); + }); - vm.open('rename', 'test-path/test'); - vm.entryName = 'test-path/test2'; - vm.submitForm(); + describe('when renaming and file has been deleted', () => { + beforeEach(async () => { + mountComponent(); + open('rename', 'src/parent_dir/foo.js'); + + await nextTick(); + + findInput().setValue('src/deleted.js'); + triggerSubmit(); + }); + + it('does not create flash', () => { expect(createFlash).not.toHaveBeenCalled(); }); - it('removes leading/trailing found in the new name', () => { - vm.open('rename', 'test-path/test'); - - vm.entryName = 'test-path /test'; - - vm.submitForm(); - - expect(vm.entryName).toBe('test-path/test'); + it('dispatches event', () => { + expect(store.dispatch).toHaveBeenCalledWith('renameEntry', { + path: 'src/parent_dir/foo.js', + name: 'deleted.js', + parentPath: 'src', + }); }); }); }); diff --git a/spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb b/spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb index 59616815de0..ac7cef20df4 100644 --- a/spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb +++ b/spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Resolvers::Ci::JobTokenScopeResolver do describe '#resolve' do context 'with access to scope' do before do - project.add_user(current_user, :maintainer) + project.add_member(current_user, :maintainer) end it 'returns nil when scope is not enabled' do @@ -51,7 +51,7 @@ RSpec.describe Resolvers::Ci::JobTokenScopeResolver do context 'without access to scope' do before do - project.add_user(current_user, :developer) + project.add_member(current_user, :developer) end it 'generates an error' do diff --git a/spec/graphql/resolvers/container_repositories_resolver_spec.rb b/spec/graphql/resolvers/container_repositories_resolver_spec.rb index d7aa761320f..ed922259903 100644 --- a/spec/graphql/resolvers/container_repositories_resolver_spec.rb +++ b/spec/graphql/resolvers/container_repositories_resolver_spec.rb @@ -62,7 +62,7 @@ RSpec.describe Resolvers::ContainerRepositoriesResolver do context 'with authorized user' do before do - group.add_user(user, :maintainer) + group.add_member(user, :maintainer) end context 'when the object is a project' do diff --git a/spec/graphql/types/ci/job_token_scope_type_spec.rb b/spec/graphql/types/ci/job_token_scope_type_spec.rb index 43225b2089b..c1a3c4dd54d 100644 --- a/spec/graphql/types/ci/job_token_scope_type_spec.rb +++ b/spec/graphql/types/ci/job_token_scope_type_spec.rb @@ -38,7 +38,7 @@ RSpec.describe GitlabSchema.types['CiJobTokenScopeType'] do context 'with access to scope' do before do - project.add_user(current_user, :maintainer) + project.add_member(current_user, :maintainer) end context 'when multiple projects in the allow list' do @@ -46,7 +46,7 @@ RSpec.describe GitlabSchema.types['CiJobTokenScopeType'] do context 'when linked projects are readable' do before do - link.target_project.add_user(current_user, :developer) + link.target_project.add_member(current_user, :developer) end it 'returns readable projects in scope' do diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index cf16807723b..b6efced698d 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -471,21 +471,60 @@ RSpec.describe DiffHelper do describe '#conflicts' do let(:merge_request) { instance_double(MergeRequest) } + let(:merge_ref_head_diff) { true } + let(:can_be_resolved_in_ui?) { true } + let(:allow_tree_conflicts) { false } + let(:files) { [instance_double(Gitlab::Conflict::File, path: 'a')] } + let(:exception) { nil } before do allow(helper).to receive(:merge_request).and_return(merge_request) - allow(helper).to receive(:options).and_return(merge_ref_head_diff: true) + allow(helper).to receive(:options).and_return(merge_ref_head_diff: merge_ref_head_diff) + + allow_next_instance_of(MergeRequests::Conflicts::ListService, merge_request, allow_tree_conflicts: allow_tree_conflicts) do |svc| + allow(svc).to receive(:can_be_resolved_in_ui?).and_return(can_be_resolved_in_ui?) + + if exception.present? + allow(svc).to receive_message_chain(:conflicts, :files).and_raise(exception) + else + allow(svc).to receive_message_chain(:conflicts, :files).and_return(files) + end + end + end + + it 'returns list of conflicts indexed by path' do + expect(helper.conflicts).to eq('a' => files.first) + end + + context 'when merge_ref_head_diff option is false' do + let(:merge_ref_head_diff) { false } + + it 'returns nil' do + expect(helper.conflicts).to be_nil + end + end + + context 'when conflicts cannot be resolved in UI' do + let(:can_be_resolved_in_ui?) { false } + + it 'returns nil' do + expect(helper.conflicts).to be_nil + end + + context 'when allow_tree_conflicts is true' do + let(:allow_tree_conflicts) { true } + + it 'returns list of conflicts' do + expect(helper.conflicts(allow_tree_conflicts: allow_tree_conflicts)).to eq('a' => files.first) + end + end end context 'when Gitlab::Git::Conflict::Resolver::ConflictSideMissing exception is raised' do - before do - allow_next_instance_of(MergeRequests::Conflicts::ListService, merge_request, allow_tree_conflicts: true) do |svc| - allow(svc).to receive_message_chain(:conflicts, :files).and_raise(Gitlab::Git::Conflict::Resolver::ConflictSideMissing) - end - end + let(:exception) { Gitlab::Git::Conflict::Resolver::ConflictSideMissing } it 'returns an empty hash' do - expect(helper.conflicts(allow_tree_conflicts: true)).to eq({}) + expect(helper.conflicts).to eq({}) end end end diff --git a/spec/helpers/namespace_storage_limit_alert_helper_spec.rb b/spec/helpers/namespace_storage_limit_alert_helper_spec.rb deleted file mode 100644 index ab3cf96edef..00000000000 --- a/spec/helpers/namespace_storage_limit_alert_helper_spec.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe NamespaceStorageLimitAlertHelper do - describe '#display_namespace_storage_limit_alert!' do - it 'is defined in CE' do - expect { helper.display_namespace_storage_limit_alert! }.not_to raise_error - end - end -end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 4502729866c..42c97b61d19 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -698,7 +698,7 @@ RSpec.describe ProjectsHelper do def grant_user_access(project, user, access) case access when :developer, :maintainer - project.add_user(user, access) + project.add_member(user, access) when :owner project.namespace.update!(owner: user) end diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb index 73a454fd9f7..bbabfedc3ee 100644 --- a/spec/helpers/todos_helper_spec.rb +++ b/spec/helpers/todos_helper_spec.rb @@ -133,6 +133,16 @@ RSpec.describe TodosHelper do expect(path).to eq("/#{todo.project.full_path}/-/work_items/#{todo.target.id}") end end + + context 'when given an issue with a note anchor' do + let(:todo) { create(:todo, project: issue.project, target: issue, note: note) } + + it 'responds with an appropriate path' do + path = helper.todo_target_path(todo) + + expect(path).to eq("/#{issue.project.full_path}/-/issues/#{issue.iid}##{dom_id(note)}") + end + end end describe '#todo_target_type_name' do diff --git a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb index 3459784708f..e8ef4e7f6e3 100644 --- a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Banzai::ReferenceParser::SnippetParser do end before do - project.add_user(project_member, :developer) + project.add_member(project_member, :developer) end describe '#nodes_visible_to_user' do diff --git a/spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb b/spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb index ec394bb9f05..34d5158a5ab 100644 --- a/spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb +++ b/spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do describe '#serialized_records' do shared_context 'when records are loaded by maintainer' do before do - project.add_user(user, Gitlab::Access::DEVELOPER) + project.add_member(user, Gitlab::Access::DEVELOPER) end it 'returns all records' do @@ -72,7 +72,7 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do context 'when records are loaded by guest' do before do - project.add_user(user, Gitlab::Access::GUEST) + project.add_member(user, Gitlab::Access::GUEST) end it 'filters out confidential issues' do @@ -124,7 +124,7 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do end before do - project.add_user(user, Gitlab::Access::DEVELOPER) + project.add_member(user, Gitlab::Access::DEVELOPER) stub_const('Gitlab::Analytics::CycleAnalytics::RecordsFetcher::MAX_RECORDS', 2) end diff --git a/spec/lib/gitlab/ci/variables/builder_spec.rb b/spec/lib/gitlab/ci/variables/builder_spec.rb index b0704ad7f50..8ec0846bdca 100644 --- a/spec/lib/gitlab/ci/variables/builder_spec.rb +++ b/spec/lib/gitlab/ci/variables/builder_spec.rb @@ -166,9 +166,8 @@ RSpec.describe Gitlab::Ci::Variables::Builder do allow(builder).to receive(:secret_instance_variables) { [var('J', 10), var('K', 10)] } allow(builder).to receive(:secret_group_variables) { [var('K', 11), var('L', 11)] } allow(builder).to receive(:secret_project_variables) { [var('L', 12), var('M', 12)] } - allow(job).to receive(:trigger_request) { double(user_variables: [var('M', 13), var('N', 13)]) } - allow(pipeline).to receive(:variables) { [var('N', 14), var('O', 14)] } - allow(pipeline).to receive(:pipeline_schedule) { double(job_variables: [var('O', 15), var('P', 15)]) } + allow(pipeline).to receive(:variables) { [var('M', 13), var('N', 13)] } + allow(pipeline).to receive(:pipeline_schedule) { double(job_variables: [var('N', 14), var('O', 14)]) } end it 'returns variables in order depending on resource hierarchy' do @@ -185,8 +184,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do var('K', 11), var('L', 11), var('L', 12), var('M', 12), var('M', 13), var('N', 13), - var('N', 14), var('O', 14), - var('O', 15), var('P', 15)]) + var('N', 14), var('O', 14)]) end it 'overrides duplicate keys depending on resource hierarchy' do @@ -198,7 +196,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do 'I' => '9', 'J' => '10', 'K' => '11', 'L' => '12', 'M' => '13', 'N' => '14', - 'O' => '15', 'P' => '15') + 'O' => '14') end end diff --git a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb index e676e5fe034..2878d72210e 100644 --- a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb +++ b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb @@ -109,7 +109,7 @@ RSpec.describe Gitlab::DatabaseImporters::InstanceAdministrators::CreateGroup do admin2 = create(:user, :admin) existing_group.add_owner(user) - existing_group.add_users([admin1, admin2], Gitlab::Access::MAINTAINER) + existing_group.add_members([admin1, admin2], Gitlab::Access::MAINTAINER) application_setting.instance_administrators_group_id = existing_group.id end diff --git a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb index 03f522ae490..3f73a730744 100644 --- a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb @@ -171,4 +171,27 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do expect(described_class.batch_size(exportable)).to eq(described_class::BATCH_SIZE) end end + + describe '#serialize_relation' do + context 'when record is a merge request' do + let(:json_writer) do + Class.new do + def write_relation_array(_, _, enumerator) + enumerator.each { _1 } + end + end.new + end + + it 'removes cached external diff' do + merge_request = create(:merge_request, source_project: exportable, target_project: exportable) + cache_dir = merge_request.merge_request_diff.send(:external_diff_cache_dir) + + expect(subject).to receive(:remove_cached_external_diff).with(merge_request).twice + + subject.serialize_relation({ merge_requests: { include: [] } }) + + expect(Dir.exist?(cache_dir)).to eq(false) + end + end + end end diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 87ca899a87d..d7ad34255c1 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -258,7 +258,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end before do - group.add_users([user, user2], GroupMember::DEVELOPER) + group.add_members([user, user2], GroupMember::DEVELOPER) end it 'maps the project member' do @@ -281,7 +281,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end before do - group.add_users([user, user2], GroupMember::DEVELOPER) + group.add_members([user, user2], GroupMember::DEVELOPER) end it 'maps the importer' do @@ -315,7 +315,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do shared_examples_for 'it fetches the access level from parent group' do before do - group.add_users([user], group_access_level) + group.add_members([user], group_access_level) end it "and resolves it correctly" do diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index 95a995b1d5e..fac960d8df2 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -875,7 +875,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do context 'with group visibility' do before do group = create(:group, visibility_level: group_visibility) - group.add_users([user], GroupMember::MAINTAINER) + group.add_members([user], GroupMember::MAINTAINER) project.update!(group: group) end diff --git a/spec/lib/gitlab/quick_actions/users_extractor_spec.rb b/spec/lib/gitlab/quick_actions/users_extractor_spec.rb new file mode 100644 index 00000000000..d00f52bb056 --- /dev/null +++ b/spec/lib/gitlab/quick_actions/users_extractor_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::QuickActions::UsersExtractor do + subject(:extractor) { described_class.new(current_user, project: project, group: group, target: target, text: text) } + + let_it_be(:current_user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:target) { create(:issue, project: project) } + + let_it_be(:pancakes) { create(:user, username: 'pancakes') } + let_it_be(:waffles) { create(:user, username: 'waffles') } + let_it_be(:syrup) { create(:user, username: 'syrup') } + + before do + allow(target).to receive(:allows_multiple_assignees?).and_return(false) + end + + context 'when the text is nil' do + let(:text) { nil } + + it 'returns an empty array' do + expect(extractor.execute).to be_empty + end + end + + context 'when the text is blank' do + let(:text) { ' ' } + + it 'returns an empty array' do + expect(extractor.execute).to be_empty + end + end + + context 'when there are users to be found' do + context 'when using usernames' do + let(:text) { 'me, pancakes waffles and syrup' } + + it 'finds the users' do + expect(extractor.execute).to contain_exactly(current_user, pancakes, waffles, syrup) + end + end + + context 'when there are too many users' do + let(:text) { 'me, pancakes waffles and syrup' } + + before do + stub_const("#{described_class}::MAX_QUICK_ACTION_USERS", 2) + end + + it 'complains' do + expect { extractor.execute }.to raise_error(described_class::TooManyError) + end + end + + context 'when using references' do + let(:text) { 'me, @pancakes @waffles and @syrup' } + + it 'finds the users' do + expect(extractor.execute).to contain_exactly(current_user, pancakes, waffles, syrup) + end + end + + context 'when using a mixture of usernames and references' do + let(:text) { 'me, @pancakes waffles and @syrup' } + + it 'finds the users' do + expect(extractor.execute).to contain_exactly(current_user, pancakes, waffles, syrup) + end + end + + context 'when one or more users cannot be found' do + let(:text) { 'me, @bacon @pancakes, chicken waffles and @syrup' } + + it 'reports an error' do + expect { extractor.execute }.to raise_error(described_class::MissingError, include('bacon', 'chicken')) + end + end + + context 'when trying to find group members' do + let(:group) { create(:group, path: 'breakfast-foods') } + let(:text) { group.to_reference } + + it 'reports an error' do + [pancakes, waffles].each { group.add_developer(_1) } + + expect { extractor.execute }.to raise_error(described_class::MissingError, include('breakfast-foods')) + end + end + end +end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index b1de3e21b77..1ae45d41f2d 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -219,19 +219,19 @@ RSpec.describe Gitlab::UserAccess do describe '#can_create_tag?' do describe 'push to none protected tag' do it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_create_tag?('random_tag')).to be_truthy end it 'returns true if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_create_tag?('random_tag')).to be_truthy end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_create_tag?('random_tag')).to be_falsey end @@ -242,19 +242,19 @@ RSpec.describe Gitlab::UserAccess do let(:not_existing_tag) { create :protected_tag, project: project } it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_create_tag?(tag.name)).to be_truthy end it 'returns false if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_create_tag?(tag.name)).to be_falsey end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_create_tag?(tag.name)).to be_falsey end @@ -266,19 +266,19 @@ RSpec.describe Gitlab::UserAccess do end it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_create_tag?(@tag.name)).to be_truthy end it 'returns true if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_create_tag?(@tag.name)).to be_truthy end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_create_tag?(@tag.name)).to be_falsey end @@ -288,19 +288,19 @@ RSpec.describe Gitlab::UserAccess do describe '#can_delete_branch?' do describe 'delete unprotected branch' do it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_delete_branch?('random_branch')).to be_truthy end it 'returns true if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_delete_branch?('random_branch')).to be_truthy end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_delete_branch?('random_branch')).to be_falsey end @@ -310,19 +310,19 @@ RSpec.describe Gitlab::UserAccess do let(:branch) { create(:protected_branch, project: project, name: "test") } it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_delete_branch?(branch.name)).to be_truthy end it 'returns false if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_delete_branch?(branch.name)).to be_falsey end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_delete_branch?(branch.name)).to be_falsey end @@ -334,7 +334,7 @@ RSpec.describe Gitlab::UserAccess do context 'when user cannot push_code to a project repository (eg. as a guest)' do it 'is false' do - project.add_user(user, :guest) + project.add_member(user, :guest) expect(access.can_push_for_ref?(ref)).to be_falsey end @@ -342,7 +342,7 @@ RSpec.describe Gitlab::UserAccess do context 'when user can push_code to a project repository (eg. as a developer)' do it 'is true' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_push_for_ref?(ref)).to be_truthy end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index a9796c28870..f501488bb8c 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -1550,7 +1550,7 @@ RSpec.describe Notify do end describe 'invitations' do - let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } + let(:owner) { create(:user).tap { |u| group.add_member(u, Gitlab::Access::OWNER) } } let(:group_member) { invite_to_group(group, inviter: inviter) } let(:inviter) { owner } @@ -1605,7 +1605,7 @@ RSpec.describe Notify do end describe 'group invitation reminders' do - let_it_be(:inviter) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } + let_it_be(:inviter) { create(:user).tap { |u| group.add_member(u, Gitlab::Access::OWNER) } } let(:group_member) { invite_to_group(group, inviter: inviter) } @@ -1688,7 +1688,7 @@ RSpec.describe Notify do describe 'group invitation accepted' do let(:invited_user) { create(:user, name: 'invited user') } - let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } + let(:owner) { create(:user).tap { |u| group.add_member(u, Gitlab::Access::OWNER) } } let(:group_member) do invitee = invite_to_group(group, inviter: owner) invitee.accept_invite!(invited_user) @@ -1714,7 +1714,7 @@ RSpec.describe Notify do end describe 'group invitation declined' do - let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } + let(:owner) { create(:user).tap { |u| group.add_member(u, Gitlab::Access::OWNER) } } let(:group_member) do invitee = invite_to_group(group, inviter: owner) invitee.decline_invite! diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d3501d9bb1b..fef28e7c550 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3251,10 +3251,6 @@ RSpec.describe Ci::Build do let(:trigger) { create(:ci_trigger, project: project) } let(:trigger_request) { create(:ci_trigger_request, pipeline: pipeline, trigger: trigger) } - let(:user_trigger_variable) do - { key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1', public: false, masked: false } - end - let(:predefined_trigger_variable) do { key: 'CI_PIPELINE_TRIGGERED', value: 'true', public: true, masked: false } end @@ -3263,26 +3259,7 @@ RSpec.describe Ci::Build do build.trigger_request = trigger_request end - shared_examples 'returns variables for triggers' do - it { is_expected.to include(user_trigger_variable) } - it { is_expected.to include(predefined_trigger_variable) } - end - - context 'when variables are stored in trigger_request' do - before do - trigger_request.update_attribute(:variables, { 'TRIGGER_KEY_1' => 'TRIGGER_VALUE_1' } ) - end - - it_behaves_like 'returns variables for triggers' - end - - context 'when variables are stored in pipeline_variables' do - before do - create(:ci_pipeline_variable, pipeline: pipeline, key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1') - end - - it_behaves_like 'returns variables for triggers' - end + it { is_expected.to include(predefined_trigger_variable) } end context 'when pipeline has a variable' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d47f43a630d..91ba12a16d5 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -726,7 +726,7 @@ RSpec.describe Group do context 'when user is a member of private group' do before do - private_group.add_user(user, Gitlab::Access::DEVELOPER) + private_group.add_member(user, Gitlab::Access::DEVELOPER) end it { is_expected.to match_array([private_group, internal_group, group]) } @@ -736,7 +736,7 @@ RSpec.describe Group do let!(:private_subgroup) { create(:group, :private, parent: private_group) } before do - private_subgroup.add_user(user, Gitlab::Access::DEVELOPER) + private_subgroup.add_member(user, Gitlab::Access::DEVELOPER) end it { is_expected.to match_array([private_subgroup, internal_group, group]) } @@ -848,7 +848,7 @@ RSpec.describe Group do expect(member).to receive(:refresh_member_authorized_projects).with(blocking: true) end - group.add_user(user, GroupMember::MAINTAINER) + group.add_member(user, GroupMember::MAINTAINER) expect(group.group_members.maintainers.map(&:user)).to include(user) end @@ -858,7 +858,7 @@ RSpec.describe Group do expect(member).to receive(:refresh_member_authorized_projects).with(blocking: false) end - group.add_user(user, GroupMember::MAINTAINER, blocking_refresh: false) + group.add_member(user, GroupMember::MAINTAINER, blocking_refresh: false) end end @@ -866,12 +866,12 @@ RSpec.describe Group do let(:user) { create(:user) } before do - group.add_users([user.id], GroupMember::GUEST) + group.add_members([user.id], GroupMember::GUEST) end it "updates the group permission" do expect(group.group_members.guests.map(&:user)).to include(user) - group.add_users([user.id], GroupMember::DEVELOPER) + group.add_members([user.id], GroupMember::DEVELOPER) expect(group.group_members.developers.map(&:user)).to include(user) expect(group.group_members.guests.map(&:user)).not_to include(user) end @@ -880,7 +880,7 @@ RSpec.describe Group do let!(:project) { create(:project, group: group) } before do - group.add_users([create(:user)], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) + group.add_members([create(:user)], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) end it 'creates a member_task with the correct attributes', :aggregate_failures do @@ -896,7 +896,7 @@ RSpec.describe Group do let(:user) { create(:user) } before do - group.add_user(user, GroupMember::MAINTAINER) + group.add_member(user, GroupMember::MAINTAINER) end it "is true if avatar is image" do @@ -993,7 +993,7 @@ RSpec.describe Group do context 'there is also a project_bot owner' do before do - group.add_user(create(:user, :project_bot), GroupMember::OWNER) + group.add_member(create(:user, :project_bot), GroupMember::OWNER) end it { expect(group.last_owner?(@members[:owner])).to be_truthy } @@ -1024,7 +1024,7 @@ RSpec.describe Group do let(:member) { blocked_user.group_members.last } before do - group.add_user(blocked_user, GroupMember::OWNER) + group.add_member(blocked_user, GroupMember::OWNER) end context 'when last_blocked_owner is set' do @@ -1050,7 +1050,7 @@ RSpec.describe Group do context 'with another active owner' do before do - group.add_user(create(:user), GroupMember::OWNER) + group.add_member(create(:user), GroupMember::OWNER) end it { expect(group.member_last_blocked_owner?(member)).to be(false) } @@ -1058,7 +1058,7 @@ RSpec.describe Group do context 'with 2 blocked owners' do before do - group.add_user(create(:user, :blocked), GroupMember::OWNER) + group.add_member(create(:user, :blocked), GroupMember::OWNER) end it { expect(group.member_last_blocked_owner?(member)).to be(false) } @@ -1082,7 +1082,7 @@ RSpec.describe Group do describe '#single_blocked_owner?' do context 'when there is only one blocked owner' do before do - group.add_user(blocked_user, GroupMember::OWNER) + group.add_member(blocked_user, GroupMember::OWNER) end it 'returns true' do @@ -1094,8 +1094,8 @@ RSpec.describe Group do let_it_be(:blocked_user_2) { create(:user, :blocked) } before do - group.add_user(blocked_user, GroupMember::OWNER) - group.add_user(blocked_user_2, GroupMember::OWNER) + group.add_member(blocked_user, GroupMember::OWNER) + group.add_member(blocked_user_2, GroupMember::OWNER) end it 'returns true' do @@ -1114,8 +1114,8 @@ RSpec.describe Group do let_it_be(:user) { create(:user) } before do - group.add_user(blocked_user, GroupMember::OWNER) - group.add_user(user, GroupMember::OWNER) + group.add_member(blocked_user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end it 'has only blocked owners' do @@ -1129,7 +1129,7 @@ RSpec.describe Group do context 'when there is only one owner' do let!(:owner) do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end it 'returns the owner' do @@ -1138,7 +1138,7 @@ RSpec.describe Group do context 'and there is also a project_bot owner' do before do - group.add_user(create(:user, :project_bot), GroupMember::OWNER) + group.add_member(create(:user, :project_bot), GroupMember::OWNER) end it 'returns only the human owner' do @@ -1151,11 +1151,11 @@ RSpec.describe Group do let_it_be(:user_2) { create(:user) } let!(:owner) do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end let!(:owner2) do - group.add_user(user_2, GroupMember::OWNER) + group.add_member(user_2, GroupMember::OWNER) end it 'returns both owners' do @@ -1164,7 +1164,7 @@ RSpec.describe Group do context 'and there is also a project_bot owner' do before do - group.add_user(create(:user, :project_bot), GroupMember::OWNER) + group.add_member(create(:user, :project_bot), GroupMember::OWNER) end it 'returns only the human owners' do @@ -1186,7 +1186,7 @@ RSpec.describe Group do let(:member) { group.members.last } before do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end context 'when last_owner is set' do @@ -1284,11 +1284,11 @@ RSpec.describe Group do requester: create(:user) } - group.add_user(members[:owner], GroupMember::OWNER) - group.add_user(members[:maintainer], GroupMember::MAINTAINER) - group.add_user(members[:developer], GroupMember::DEVELOPER) - group.add_user(members[:reporter], GroupMember::REPORTER) - group.add_user(members[:guest], GroupMember::GUEST) + group.add_member(members[:owner], GroupMember::OWNER) + group.add_member(members[:maintainer], GroupMember::MAINTAINER) + group.add_member(members[:developer], GroupMember::DEVELOPER) + group.add_member(members[:reporter], GroupMember::REPORTER) + group.add_member(members[:guest], GroupMember::GUEST) group.request_access(members[:requester]) members @@ -1464,8 +1464,8 @@ RSpec.describe Group do describe '#direct_members' do let_it_be(:group) { create(:group, :nested) } - let_it_be(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } - let_it_be(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + let_it_be(:maintainer) { group.parent.add_member(create(:user), GroupMember::MAINTAINER) } + let_it_be(:developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } it 'does not return members of the parent' do expect(group.direct_members).not_to include(maintainer) @@ -1491,8 +1491,8 @@ RSpec.describe Group do shared_examples_for 'members_with_parents' do let!(:group) { create(:group, :nested) } - let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } - let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + let!(:maintainer) { group.parent.add_member(create(:user), GroupMember::MAINTAINER) } + let!(:developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } let!(:pending_maintainer) { create(:group_member, :awaiting, :maintainer, group: group.parent) } let!(:pending_developer) { create(:group_member, :awaiting, :developer, group: group) } @@ -1603,9 +1603,9 @@ RSpec.describe Group do context 'members-related methods' do let!(:group) { create(:group, :nested) } let!(:sub_group) { create(:group, parent: group) } - let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } - let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } - let!(:other_developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + let!(:maintainer) { group.parent.add_member(create(:user), GroupMember::MAINTAINER) } + let!(:developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } + let!(:other_developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } describe '#direct_and_indirect_members' do it 'returns parents members' do @@ -1619,7 +1619,7 @@ RSpec.describe Group do end describe '#direct_and_indirect_members_with_inactive' do - let!(:maintainer_blocked) { group.parent.add_user(create(:user, :blocked), GroupMember::MAINTAINER) } + let!(:maintainer_blocked) { group.parent.add_member(create(:user, :blocked), GroupMember::MAINTAINER) } it 'returns parents members' do expect(group.direct_and_indirect_members_with_inactive).to include(developer) @@ -1795,8 +1795,8 @@ RSpec.describe Group do maintainer = create(:user) developer = create(:user) - group.add_user(maintainer, GroupMember::MAINTAINER) - group.add_user(developer, GroupMember::DEVELOPER) + group.add_member(maintainer, GroupMember::MAINTAINER) + group.add_member(developer, GroupMember::DEVELOPER) expect(group.user_ids_for_project_authorizations) .to include(maintainer.id, developer.id) @@ -1847,7 +1847,7 @@ RSpec.describe Group do context 'group membership' do before do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end it 'is called when require_two_factor_authentication is changed' do @@ -1870,7 +1870,7 @@ RSpec.describe Group do it 'calls #update_two_factor_requirement on each group member' do other_user = create(:user) - group.add_user(other_user, GroupMember::OWNER) + group.add_member(other_user, GroupMember::OWNER) calls = 0 allow_any_instance_of(User).to receive(:update_two_factor_requirement) do @@ -1885,7 +1885,7 @@ RSpec.describe Group do context 'sub groups and projects' do it 'enables two_factor_requirement for group member' do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) group.update!(require_two_factor_authentication: true) @@ -1899,7 +1899,7 @@ RSpec.describe Group do context 'two_factor_requirement is also enabled for ancestor group' do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) - subgroup.add_user(indirect_user, GroupMember::OWNER) + subgroup.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: true) @@ -1910,7 +1910,7 @@ RSpec.describe Group do context 'two_factor_requirement is disabled for ancestor group' do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group, require_two_factor_authentication: true) - subgroup.add_user(indirect_user, GroupMember::OWNER) + subgroup.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: false) @@ -1919,7 +1919,7 @@ RSpec.describe Group do it 'enable two_factor_requirement for ancestor group member' do ancestor_group = create(:group) - ancestor_group.add_user(indirect_user, GroupMember::OWNER) + ancestor_group.add_member(indirect_user, GroupMember::OWNER) group.update!(parent: ancestor_group) group.update!(require_two_factor_authentication: true) @@ -1933,7 +1933,7 @@ RSpec.describe Group do context 'two_factor_requirement is enabled for ancestor group' do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) - subgroup.add_user(indirect_user, GroupMember::OWNER) + subgroup.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: true) @@ -1944,7 +1944,7 @@ RSpec.describe Group do context 'two_factor_requirement is also disabled for ancestor group' do it 'disables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) - subgroup.add_user(indirect_user, GroupMember::OWNER) + subgroup.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: false) @@ -1954,7 +1954,7 @@ RSpec.describe Group do it 'disables two_factor_requirement for ancestor group member' do ancestor_group = create(:group, require_two_factor_authentication: false) indirect_user.update!(require_two_factor_authentication_from_group: true) - ancestor_group.add_user(indirect_user, GroupMember::OWNER) + ancestor_group.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: false) diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index f93c2d36966..94032146f51 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -219,7 +219,7 @@ RSpec.describe GroupMember do end context 'on create' do - let(:action) { group.add_user(user, Gitlab::Access::GUEST) } + let(:action) { group.add_member(user, Gitlab::Access::GUEST) } let(:blocking) { true } it 'changes access level', :sidekiq_inline do @@ -241,7 +241,7 @@ RSpec.describe GroupMember do context 'on update' do before do - group.add_user(user, Gitlab::Access::GUEST) + group.add_member(user, Gitlab::Access::GUEST) end let(:action) { group.members.find_by(user: user).update!(access_level: Gitlab::Access::DEVELOPER) } @@ -266,7 +266,7 @@ RSpec.describe GroupMember do context 'on destroy' do before do - group.add_user(user, Gitlab::Access::GUEST) + group.add_member(user, Gitlab::Access::GUEST) end let(:action) { group.members.find_by(user: user).destroy! } diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 8c989f5aaca..48798d419e8 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -111,12 +111,12 @@ RSpec.describe ProjectMember do end end - describe '.add_users_to_projects' do + describe '.add_members_to_projects' do it 'adds the given users to the given projects' do projects = create_list(:project, 2) users = create_list(:user, 2) - described_class.add_users_to_projects( + described_class.add_members_to_projects( [projects.first.id, projects.second.id], [users.first.id, users.second], described_class::MAINTAINER) @@ -236,7 +236,7 @@ RSpec.describe ProjectMember do end context 'on create' do - let(:action) { project.add_user(user, Gitlab::Access::GUEST) } + let(:action) { project.add_member(user, Gitlab::Access::GUEST) } it 'changes access level' do expect { action }.to change { user.can?(:guest_access, project) }.from(false).to(true) @@ -250,7 +250,7 @@ RSpec.describe ProjectMember do let(:action) { project.members.find_by(user: user).update!(access_level: Gitlab::Access::DEVELOPER) } before do - project.add_user(user, Gitlab::Access::GUEST) + project.add_member(user, Gitlab::Access::GUEST) end it 'changes access level' do @@ -265,7 +265,7 @@ RSpec.describe ProjectMember do let(:action) { project.members.find_by(user: user).destroy! } before do - project.add_user(user, Gitlab::Access::GUEST) + project.add_member(user, Gitlab::Access::GUEST) end it 'changes access level', :sidekiq_inline do diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb index c9bcb900eca..64203c50759 100644 --- a/spec/models/merge_request_diff_file_spec.rb +++ b/spec/models/merge_request_diff_file_spec.rb @@ -13,50 +13,53 @@ RSpec.describe MergeRequestDiffFile do let(:invalid_items_for_bulk_insertion) { [] } # class does not have any validations defined end + let(:unpacked) { 'unpacked' } + let(:packed) { [unpacked].pack('m0') } + let(:file) { create(:merge_request).merge_request_diff.merge_request_diff_files.first } + describe '#diff' do + let(:file) { build(:merge_request_diff_file) } + context 'when diff is not stored' do let(:unpacked) { 'unpacked' } let(:packed) { [unpacked].pack('m0') } before do - subject.diff = packed + file.diff = packed end context 'when the diff is marked as binary' do before do - subject.binary = true + file.binary = true end it 'unpacks from base 64' do - expect(subject.diff).to eq(unpacked) + expect(file.diff).to eq(unpacked) end context 'invalid base64' do let(:packed) { '---/dev/null' } it 'returns the raw diff' do - expect(subject.diff).to eq(packed) + expect(file.diff).to eq(packed) end end end context 'when the diff is not marked as binary' do it 'returns the raw diff' do - expect(subject.diff).to eq(packed) + expect(file.diff).to eq(packed) end end end context 'when diff is stored in DB' do - let(:file) { create(:merge_request).merge_request_diff.merge_request_diff_files.first } - it 'returns UTF-8 string' do expect(file.diff.encoding).to eq Encoding::UTF_8 end end context 'when diff is stored in external storage' do - let(:file) { create(:merge_request).merge_request_diff.merge_request_diff_files.first } let(:test_dir) { 'tmp/tests/external-diffs' } around do |example| @@ -81,17 +84,132 @@ RSpec.describe MergeRequestDiffFile do describe '#utf8_diff' do it 'does not raise error when the diff is binary' do - subject.diff = "\x05\x00\x68\x65\x6c\x6c\x6f" + file = build(:merge_request_diff_file) + file.diff = "\x05\x00\x68\x65\x6c\x6c\x6f" - expect { subject.utf8_diff }.not_to raise_error + expect { file.utf8_diff }.not_to raise_error end it 'calls #diff once' do - allow(subject).to receive(:diff).and_return('test') + allow(file).to receive(:diff).and_return('test') - expect(subject).to receive(:diff).once + expect(file).to receive(:diff).once - subject.utf8_diff + file.utf8_diff + end + end + + describe '#diff_export' do + context 'when diff is externally stored' do + let(:test_dir) { 'tmp/tests/external-diffs' } + + around do |example| + FileUtils.mkdir_p(test_dir) + + begin + example.run + ensure + FileUtils.rm_rf(test_dir) + end + end + + before do + stub_external_diffs_setting(enabled: true, storage_path: test_dir) + end + + context 'when external diff is not cached' do + it 'caches external diffs' do + expect(file.merge_request_diff).to receive(:cache_external_diff).and_call_original + + expect(file.diff_export).to eq(file.utf8_diff) + end + end + + context 'when external diff is already cached' do + it 'reads diff from cached external diff' do + file_stub = double + + allow(file.merge_request_diff).to receive(:cached_external_diff).and_yield(file_stub) + expect(file_stub).to receive(:seek).with(file.external_diff_offset) + expect(file_stub).to receive(:read).with(file.external_diff_size) + + file.diff_export + end + end + + context 'when the diff is marked as binary' do + let(:file) { build(:merge_request_diff_file) } + + before do + allow(file.merge_request_diff).to receive(:stored_externally?).and_return(true) + allow(file.merge_request_diff).to receive(:cached_external_diff).and_return(packed) + end + + context 'when the diff is marked as binary' do + before do + file.binary = true + end + + it 'unpacks from base 64' do + expect(file.diff_export).to eq(unpacked) + end + + context 'invalid base64' do + let(:packed) { '---/dev/null' } + + it 'returns the raw diff' do + expect(file.diff_export).to eq(packed) + end + end + end + + context 'when the diff is not marked as binary' do + it 'returns the raw diff' do + expect(file.diff_export).to eq(packed) + end + end + end + + context 'when content responds to #encoding' do + it 'encodes content to utf8 encoding' do + expect(file.diff_export.encoding).to eq(Encoding::UTF_8) + end + end + + context 'when content is blank' do + it 'returns an empty string' do + allow(file.merge_request_diff).to receive(:cached_external_diff).and_return(nil) + + expect(file.diff_export).to eq('') + end + end + + context 'when exception is raised' do + it 'falls back to #utf8_diff' do + allow(file).to receive(:binary?).and_raise(StandardError) + expect(file).to receive(:utf8_diff) + + file.diff_export + end + end + end + + context 'when externally_stored_diffs_caching_export feature flag is disabled' do + it 'calls #utf8_diff' do + stub_feature_flags(externally_stored_diffs_caching_export: false) + + expect(file).to receive(:utf8_diff) + + file.diff_export + end + end + + context 'when diff is not stored externally' do + it 'calls #utf8_diff' do + expect(file).to receive(:utf8_diff) + + file.diff_export + end end end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index afe7251f59a..007e84164a8 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -1120,4 +1120,101 @@ RSpec.describe MergeRequestDiff do expect(described_class.latest_diff_for_merge_requests(nil)).to be_empty end end + + context 'external diff caching' do + let(:test_dir) { 'tmp/tests/external-diffs' } + let(:cache_dir) { File.join(Dir.tmpdir, "project-#{diff.project.id}-external-mr-#{diff.merge_request_id}-diff-#{diff.id}-cache") } + let(:cache_filepath) { File.join(cache_dir, "diff-#{diff.id}") } + let(:external_diff_content) { diff.opening_external_diff { |diff| diff.read } } + + around do |example| + FileUtils.mkdir_p(test_dir) + + begin + example.run + ensure + FileUtils.rm_rf(test_dir) + end + end + + before do + stub_external_diffs_setting(enabled: true, storage_path: test_dir) + end + + subject(:diff) { diff_with_commits } + + describe '#cached_external_diff' do + context 'when diff is externally stored' do + context 'when diff is already cached' do + it 'yields cached file' do + Dir.mkdir(cache_dir) + File.open(cache_filepath, 'wb') { |f| f.write(external_diff_content) } + + expect(diff).not_to receive(:cache_external_diff) + + expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(File) + end + end + + context 'when diff is not cached' do + it 'caches external diff in tmp storage' do + expect(diff).to receive(:cache_external_diff).and_call_original + expect(File.exist?(cache_filepath)).to eq(false) + expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(File) + expect(File.exist?(cache_filepath)).to eq(true) + expect(File.read(cache_filepath)).to eq(external_diff_content) + end + end + end + + context 'when diff is not externally stored' do + it 'yields nil' do + stub_external_diffs_setting(enabled: false) + + expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(nil) + end + end + end + + describe '#remove_cached_external_diff' do + before do + diff.cached_external_diff { |diff| diff } + end + + it 'removes external diff cache diff' do + expect(Dir.exist?(cache_dir)).to eq(true) + + diff.remove_cached_external_diff + + expect(Dir.exist?(cache_dir)).to eq(false) + end + + context 'when path is traversed' do + it 'raises' do + allow(diff).to receive(:external_diff_cache_dir).and_return(File.join(cache_dir, '..')) + + expect { diff.remove_cached_external_diff }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + end + end + + context 'when path is not allowed' do + it 'raises' do + allow(diff).to receive(:external_diff_cache_dir).and_return('/') + + expect { diff.remove_cached_external_diff }.to raise_error(StandardError, 'path / is not allowed') + end + end + + context 'when dir does not exist' do + it 'returns' do + FileUtils.rm_rf(cache_dir) + + expect(Dir.exist?(cache_dir)).to eq(false) + expect(FileUtils).not_to receive(:rm_rf).with(cache_dir) + + diff.remove_cached_external_diff + end + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 77b81b2c310..c2f3deb6c06 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -658,7 +658,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end before do - project.add_user(user, :developer) + project.add_member(user, :developer) end describe '.total_time_to_merge' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b7ed7b20833..74a4a023a20 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -821,7 +821,7 @@ RSpec.describe Project, factory_default: :keep do end describe 'delegation' do - [:add_guest, :add_reporter, :add_developer, :add_maintainer, :add_user, :add_users].each do |method| + [:add_guest, :add_reporter, :add_developer, :add_maintainer, :add_member, :add_members].each do |method| it { is_expected.to delegate_method(method).to(:team) } end @@ -1862,7 +1862,7 @@ RSpec.describe Project, factory_default: :keep do describe 'when a user has access to a project' do before do - project.add_user(user, Gitlab::Access::MAINTAINER) + project.add_member(user, Gitlab::Access::MAINTAINER) end it { is_expected.to eq([project]) } diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 2ddbab7779e..1fab07c1452 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -251,13 +251,13 @@ RSpec.describe ProjectTeam do end end - describe '#add_users' do + describe '#add_members' do let(:user1) { create(:user) } let(:user2) { create(:user) } let(:project) { create(:project) } it 'add the given users to the team' do - project.team.add_users([user1, user2], :reporter) + project.team.add_members([user1, user2], :reporter) expect(project.team.reporter?(user1)).to be(true) expect(project.team.reporter?(user2)).to be(true) @@ -265,7 +265,7 @@ RSpec.describe ProjectTeam do context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do before do - project.team.add_users([user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) + project.team.add_members([user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) end it 'creates a member_task with the correct attributes', :aggregate_failures do @@ -277,12 +277,12 @@ RSpec.describe ProjectTeam do end end - describe '#add_user' do + describe '#add_member' do let(:user) { create(:user) } let(:project) { create(:project) } it 'add the given user to the team' do - project.team.add_user(user, :reporter) + project.team.add_member(user, :reporter) expect(project.team.reporter?(user)).to be(true) end diff --git a/spec/models/ssh_host_key_spec.rb b/spec/models/ssh_host_key_spec.rb index 4b756846598..0348aab9f97 100644 --- a/spec/models/ssh_host_key_spec.rb +++ b/spec/models/ssh_host_key_spec.rb @@ -26,6 +26,9 @@ RSpec.describe SshHostKey do 'Ebi86VjJRi2sOuYoXQU1' end + let(:ssh_key1) { Gitlab::SSHPublicKey.new(key1) } + let(:ssh_key2) { Gitlab::SSHPublicKey.new(key2) } + # Purposefully ordered so that `sort` will make changes let(:known_hosts) do <<~EOF @@ -88,10 +91,17 @@ RSpec.describe SshHostKey do it 'returns an array of indexed fingerprints when the cache is filled' do stub_reactive_cache(ssh_host_key, known_hosts: known_hosts) - expected = [key1, key2] - .map { |data| Gitlab::SSHPublicKey.new(data) } + expected = [ssh_key1, ssh_key2] .each_with_index - .map { |key, i| { bits: key.bits, fingerprint: key.fingerprint, type: key.type, index: i } } + .map do |key, i| + { + bits: key.bits, + fingerprint: key.fingerprint, + fingerprint_sha256: key.fingerprint_sha256, + type: key.type, + index: i + } + end expect(ssh_host_key.fingerprints.as_json).to eq(expected) end @@ -107,8 +117,16 @@ RSpec.describe SshHostKey do expect(ssh_host_key.fingerprints.as_json).to eq( [ - { bits: 2048, fingerprint: Gitlab::SSHPublicKey.new(key1).fingerprint, type: :rsa, index: 0 }, - { bits: 2048, fingerprint: Gitlab::SSHPublicKey.new(key2).fingerprint, type: :rsa, index: 1 } + { bits: 2048, + fingerprint: ssh_key1.fingerprint, + fingerprint_sha256: ssh_key1.fingerprint_sha256, + type: :rsa, + index: 0 }, + { bits: 2048, + fingerprint: ssh_key2.fingerprint, + fingerprint_sha256: ssh_key2.fingerprint_sha256, + type: :rsa, + index: 1 } ] ) end @@ -116,6 +134,19 @@ RSpec.describe SshHostKey do it 'returns an empty array when the cache is empty' do expect(ssh_host_key.fingerprints).to eq([]) end + + context 'when FIPS is enabled', :fips_mode do + it 'only includes SHA256 fingerprint' do + stub_reactive_cache(ssh_host_key, known_hosts: known_hosts) + + expect(ssh_host_key.fingerprints.as_json).to eq( + [ + { bits: 2048, fingerprint_sha256: ssh_key1.fingerprint_sha256, type: :rsa, index: 0 }, + { bits: 2048, fingerprint_sha256: ssh_key2.fingerprint_sha256, type: :rsa, index: 1 } + ] + ) + end + end end describe '#host_keys_changed?' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7a5908fb586..626d8c8d337 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2292,7 +2292,7 @@ RSpec.describe User do @group = create :group @group.add_owner(@user) - @group.add_user(@user2, GroupMember::OWNER) + @group.add_member(@user2, GroupMember::OWNER) end it { expect(@user2.several_namespaces?).to be_truthy } @@ -3834,7 +3834,7 @@ RSpec.describe User do let!(:project) { create(:project, group: project_group) } before do - private_group.add_user(user, Gitlab::Access::MAINTAINER) + private_group.add_member(user, Gitlab::Access::MAINTAINER) project.add_maintainer(user) end @@ -3861,7 +3861,7 @@ RSpec.describe User do let_it_be(:parent_group) do create(:group).tap do |g| - g.add_user(user, Gitlab::Access::MAINTAINER) + g.add_member(user, Gitlab::Access::MAINTAINER) end end @@ -4289,7 +4289,7 @@ RSpec.describe User do let!(:runner) { create(:ci_runner, :group, groups: [group]) } def add_user(access) - group.add_user(user, access) + group.add_member(user, access) end it_behaves_like :group_member @@ -4379,7 +4379,7 @@ RSpec.describe User do let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } def add_user(access) - project.add_user(user, access) + project.add_member(user, access) end it_behaves_like :project_member @@ -4401,8 +4401,8 @@ RSpec.describe User do let!(:another_user) { create(:user) } def add_user(access) - subgroup.add_user(user, access) - group.add_user(another_user, :owner) + subgroup.add_member(user, access) + group.add_member(another_user, :owner) end it_behaves_like :group_member @@ -4759,8 +4759,8 @@ RSpec.describe User do let(:group2) { create :group, require_two_factor_authentication: true, two_factor_grace_period: 32 } before do - group1.add_user(user, GroupMember::OWNER) - group2.add_user(user, GroupMember::OWNER) + group1.add_member(user, GroupMember::OWNER) + group2.add_member(user, GroupMember::OWNER) user.update_two_factor_requirement end @@ -4779,7 +4779,7 @@ RSpec.describe User do let!(:group1a) { create :group, parent: group1 } before do - group1a.add_user(user, GroupMember::OWNER) + group1a.add_member(user, GroupMember::OWNER) user.update_two_factor_requirement end @@ -4794,7 +4794,7 @@ RSpec.describe User do let!(:group1a) { create :group, require_two_factor_authentication: true, parent: group1 } before do - group1.add_user(user, GroupMember::OWNER) + group1.add_member(user, GroupMember::OWNER) user.update_two_factor_requirement end @@ -4815,7 +4815,7 @@ RSpec.describe User do group_access: ProjectGroupLink.default_access ) - group2.add_user(user, GroupMember::OWNER) + group2.add_member(user, GroupMember::OWNER) end it 'does not require 2FA' do @@ -4829,7 +4829,7 @@ RSpec.describe User do let(:group) { create :group } before do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) user.update_two_factor_requirement end @@ -4858,8 +4858,8 @@ RSpec.describe User do let(:user) { create :user } before do - group.add_user(user, GroupMember::OWNER) - group_not_requiring_2FA.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) + group_not_requiring_2FA.add_member(user, GroupMember::OWNER) end context 'when user is direct member of group requiring 2FA' do diff --git a/spec/policies/environment_policy_spec.rb b/spec/policies/environment_policy_spec.rb index 649b1a770c0..701fc7ac9ae 100644 --- a/spec/policies/environment_policy_spec.rb +++ b/spec/policies/environment_policy_spec.rb @@ -28,7 +28,7 @@ RSpec.describe EnvironmentPolicy do with_them do before do - project.add_user(user, access_level) unless access_level.nil? + project.add_member(user, access_level) unless access_level.nil? end it { expect(policy.allowed?(:stop_environment)).to be allowed? } @@ -49,7 +49,7 @@ RSpec.describe EnvironmentPolicy do context 'with protected branch' do with_them do before do - project.add_user(user, access_level) unless access_level.nil? + project.add_member(user, access_level) unless access_level.nil? create(:protected_branch, :no_one_can_push, name: 'master', project: project) end @@ -86,7 +86,7 @@ RSpec.describe EnvironmentPolicy do with_them do before do - project.add_user(user, access_level) unless access_level.nil? + project.add_member(user, access_level) unless access_level.nil? end it { expect(policy.allowed?(:stop_environment)).to be allowed? } @@ -120,7 +120,7 @@ RSpec.describe EnvironmentPolicy do with_them do before do - project.add_user(user, access_level) unless access_level.nil? + project.add_member(user, access_level) unless access_level.nil? end it { expect(policy).to be_disallowed :destroy_environment } diff --git a/spec/policies/namespace/root_storage_statistics_policy_spec.rb b/spec/policies/namespace/root_storage_statistics_policy_spec.rb index e6b58bca4a8..89875f83c9b 100644 --- a/spec/policies/namespace/root_storage_statistics_policy_spec.rb +++ b/spec/policies/namespace/root_storage_statistics_policy_spec.rb @@ -59,7 +59,7 @@ RSpec.describe Namespace::RootStorageStatisticsPolicy do with_them do before do - group.add_user(user, user_type) unless user_type == :non_member + group.add_member(user, user_type) unless user_type == :non_member end it { is_expected.to eq(outcome) } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 5884dc3f23f..de36b755e88 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -1736,7 +1736,7 @@ RSpec.describe ProjectPolicy do %w(guest reporter developer maintainer).each do |role| context role do before do - project.add_user(current_user, role.to_sym) + project.add_member(current_user, role.to_sym) end if role == 'guest' @@ -1770,7 +1770,7 @@ RSpec.describe ProjectPolicy do %w(guest reporter developer maintainer).each do |role| context role do before do - project.add_user(current_user, role.to_sym) + project.add_member(current_user, role.to_sym) end it { is_expected.to be_allowed(:read_ci_cd_analytics) } @@ -1800,7 +1800,7 @@ RSpec.describe ProjectPolicy do %w(guest reporter developer maintainer).each do |role| context role do before do - project.add_user(current_user, role.to_sym) + project.add_member(current_user, role.to_sym) end if role == 'guest' diff --git a/spec/policies/project_statistics_policy_spec.rb b/spec/policies/project_statistics_policy_spec.rb index 74630dc38ad..56e6161a264 100644 --- a/spec/policies/project_statistics_policy_spec.rb +++ b/spec/policies/project_statistics_policy_spec.rb @@ -72,7 +72,7 @@ RSpec.describe ProjectStatisticsPolicy do before do unless [:unauthenticated, :non_member].include?(user_type) - project.add_user(external, user_type) + project.add_member(external, user_type) end end diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 3f3a61949d4..746be1ccc44 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -675,14 +675,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end - context 'when variables are stored in trigger_request' do - before do - trigger_request.update_attribute(:variables, { TRIGGER_KEY_1: 'TRIGGER_VALUE_1' } ) - end - - it_behaves_like 'expected variables behavior' - end - context 'when variables are stored in pipeline_variables' do before do create(:ci_pipeline_variable, pipeline: pipeline, key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1') diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index 110d6e2f99e..d6c3999f22f 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -173,7 +173,7 @@ RSpec.describe API::Events do let(:second_note) { create(:note_on_issue, project: create(:project)) } before do - second_note.project.add_user(user, :developer) + second_note.project.add_member(user, :developer) [second_note].each do |note| EventCreateService.new.leave_note(note, user) diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 446d1fb1bdb..e17a83d8e47 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -424,7 +424,7 @@ RSpec.describe 'Query.runner(id)' do let(:user) { create(:user) } before do - group.add_user(user, Gitlab::Access::OWNER) + group.add_member(user, Gitlab::Access::OWNER) end it_behaves_like 'retrieval with no admin url' do diff --git a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb index 847fa72522e..14c55e61a65 100644 --- a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb +++ b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb @@ -71,7 +71,7 @@ RSpec.describe 'container repository details' do with_them do before do project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility.to_s.upcase, false)) - project.add_user(user, role) unless role == :anonymous + project.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/container_repositories_spec.rb b/spec/requests/api/graphql/group/container_repositories_spec.rb index be0b866af4a..8ec321c8d7c 100644 --- a/spec/requests/api/graphql/group/container_repositories_spec.rb +++ b/spec/requests/api/graphql/group/container_repositories_spec.rb @@ -82,7 +82,7 @@ RSpec.describe 'getting container repositories in a group' do group.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb index cdb21512894..daa1483e956 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb @@ -75,7 +75,7 @@ RSpec.describe 'getting dependency proxy blobs in a group' do with_them do before do group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb index d21c3046c1a..cc706c3051f 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb @@ -61,7 +61,7 @@ RSpec.describe 'getting dependency proxy settings for a group' do with_them do before do group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb index 40f4b082072..3b2b04b1322 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb @@ -60,7 +60,7 @@ RSpec.describe 'getting dependency proxy image ttl policy for a group' do with_them do before do group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb index c7149c100b2..9bb661a734e 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb @@ -73,7 +73,7 @@ RSpec.describe 'getting dependency proxy manifests in a group' do with_them do before do group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/project/container_repositories_spec.rb b/spec/requests/api/graphql/project/container_repositories_spec.rb index bbab6012f3f..01b117a89d8 100644 --- a/spec/requests/api/graphql/project/container_repositories_spec.rb +++ b/spec/requests/api/graphql/project/container_repositories_spec.rb @@ -81,7 +81,7 @@ RSpec.describe 'getting container repositories in a project' do with_them do before do project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility.to_s.upcase, false)) - project.add_user(user, role) unless role == :anonymous + project.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb b/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb index a025c57d4b8..33e1dbcba27 100644 --- a/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb +++ b/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb @@ -61,7 +61,7 @@ RSpec.describe 'getting the packages cleanup policy linked to a project' do with_them do before do project.update!(visibility: visibility.to_s) - project.add_user(current_user, role) unless role == :anonymous + project.add_member(current_user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 6d5676bbe35..a7b4bea362f 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -10,7 +10,7 @@ RSpec.describe API::GroupVariables do let(:access_level) {} before do - group.add_user(user, access_level) if access_level + group.add_member(user, access_level) if access_level end describe 'GET /groups/:id/variables' do diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index 64ad5733c1b..58068bf01a3 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -320,7 +320,7 @@ RSpec.describe API::Invitations do let(:source) { project } end - it 'records queries', :request_store, :use_sql_query_cache do + it 'does not exceed expected queries count for emails', :request_store, :use_sql_query_cache do post invitations_url(project, maintainer), params: { email: email, access_level: Member::DEVELOPER } control = ActiveRecord::QueryRecorder.new(skip_cached: false) do @@ -336,7 +336,25 @@ RSpec.describe API::Invitations do end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) end - it 'records queries with secondary emails', :request_store, :use_sql_query_cache do + it 'does not exceed expected queries count for user_ids', :request_store, :use_sql_query_cache do + stranger2 = create(:user) + + post invitations_url(project, maintainer), params: { user_id: stranger.id, access_level: Member::DEVELOPER } + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post invitations_url(project, maintainer), params: { user_id: stranger2.id, access_level: Member::DEVELOPER } + end + + users = create_list(:user, 5) + + unresolved_n_plus_ones = 116 # 51 for 1 vs 167 for 5 - currently there are 29 queries added per user + + expect do + post invitations_url(project, maintainer), params: { user_id: users.map(&:id).join(','), access_level: Member::DEVELOPER } + end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end + + it 'does not exceed expected queries count with secondary emails', :request_store, :use_sql_query_cache do create(:email, email: email, user: create(:user)) post invitations_url(project, maintainer), params: { email: email, access_level: Member::DEVELOPER } @@ -365,7 +383,7 @@ RSpec.describe API::Invitations do let(:source) { group } end - it 'records queries', :request_store, :use_sql_query_cache do + it 'does not exceed expected queries count for emails', :request_store, :use_sql_query_cache do post invitations_url(group, maintainer), params: { email: email, access_level: Member::DEVELOPER } control = ActiveRecord::QueryRecorder.new(skip_cached: false) do @@ -381,7 +399,7 @@ RSpec.describe API::Invitations do end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) end - it 'records queries with secondary emails', :request_store, :use_sql_query_cache do + it 'does not exceed expected queries count for secondary emails', :request_store, :use_sql_query_cache do create(:email, email: email, user: create(:user)) post invitations_url(group, maintainer), params: { email: email, access_level: Member::DEVELOPER } diff --git a/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb b/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb index ffeaf1075f3..b0c2eaec4e2 100644 --- a/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb +++ b/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb @@ -3,7 +3,40 @@ require 'spec_helper' RSpec.describe JiraConnect::OauthApplicationIdsController do + describe 'OPTIONS /-/jira_connect/oauth_application_id' do + before do + stub_application_setting(jira_connect_application_key: '123456') + + options '/-/jira_connect/oauth_application_id', headers: { 'Origin' => 'http://notgitlab.com' } + end + + it 'returns 200' do + expect(response).to have_gitlab_http_status(:ok) + end + + it 'allows cross-origin requests', :aggregate_failures do + expect(response.headers['Access-Control-Allow-Origin']).to eq '*' + expect(response.headers['Access-Control-Allow-Methods']).to eq 'GET, OPTIONS' + expect(response.headers['Access-Control-Allow-Credentials']).to be_nil + end + + context 'on GitLab.com' do + before do + allow(Gitlab).to receive(:com?).and_return(true) + end + + it 'renders not found' do + options '/-/jira_connect/oauth_application_id' + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.headers['Access-Control-Allow-Origin']).not_to eq '*' + end + end + end + describe 'GET /-/jira_connect/oauth_application_id' do + let(:cors_request_headers) { { 'Origin' => 'http://notgitlab.com' } } + before do stub_application_setting(jira_connect_application_key: '123456') end @@ -15,6 +48,14 @@ RSpec.describe JiraConnect::OauthApplicationIdsController do expect(json_response).to eq({ "application_id" => "123456" }) end + it 'allows cross-origin requests', :aggregate_failures do + get '/-/jira_connect/oauth_application_id', headers: cors_request_headers + + expect(response.headers['Access-Control-Allow-Origin']).to eq '*' + expect(response.headers['Access-Control-Allow-Methods']).to eq 'GET, OPTIONS' + expect(response.headers['Access-Control-Allow-Credentials']).to be_nil + end + context 'application ID is empty' do before do stub_application_setting(jira_connect_application_key: '') @@ -38,5 +79,17 @@ RSpec.describe JiraConnect::OauthApplicationIdsController do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'on GitLab.com' do + before do + allow(Gitlab).to receive(:com?).and_return(true) + end + + it 'renders not found' do + get '/-/jira_connect/oauth_application_id' + + expect(response).to have_gitlab_http_status(:not_found) + end + end end end diff --git a/spec/requests/jira_connect/subscriptions_controller_spec.rb b/spec/requests/jira_connect/subscriptions_controller_spec.rb new file mode 100644 index 00000000000..b10d07b3771 --- /dev/null +++ b/spec/requests/jira_connect/subscriptions_controller_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::SubscriptionsController do + describe 'GET /-/jira_connect/subscriptions' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'http://self-managed-gitlab.com') } + + let(:qsh) do + Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') + end + + let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) } + + before do + get '/-/jira_connect/subscriptions', params: { jwt: jwt } + end + + subject(:content_security_policy) { response.headers['Content-Security-Policy'] } + + it { is_expected.to include('http://self-managed-gitlab.com/-/jira_connect/oauth_application_ids')} + + context 'with no self-managed instance configured' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: '') } + + it { is_expected.not_to include('http://self-managed-gitlab.com')} + end + end +end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index c647fee1564..afaa6168bfd 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -120,9 +120,9 @@ RSpec.describe 'OpenID Connect requests' do let!(:group4) { create :group, parent: group3 } before do - group1.add_user(user, GroupMember::OWNER) - group3.add_user(user, Gitlab::Access::DEVELOPER) - group4.add_user(user, Gitlab::Access::MAINTAINER) + group1.add_member(user, GroupMember::OWNER) + group3.add_member(user, Gitlab::Access::DEVELOPER) + group4.add_member(user, Gitlab::Access::MAINTAINER) request_user_info! end @@ -163,8 +163,8 @@ RSpec.describe 'OpenID Connect requests' do let!(:group4) { create :group, parent: group3 } before do - group1.add_user(user, Gitlab::Access::OWNER) - group3.add_user(user, Gitlab::Access::DEVELOPER) + group1.add_member(user, Gitlab::Access::OWNER) + group3.add_member(user, Gitlab::Access::DEVELOPER) request_access_token! @payload = JSON::JWT.decode(json_response['id_token'], :skip_verification) @@ -358,8 +358,8 @@ RSpec.describe 'OpenID Connect requests' do let!(:group4) { create :group, parent: group3 } before do - group1.add_user(user, Gitlab::Access::OWNER) - group3.add_user(user, Gitlab::Access::DEVELOPER) + group1.add_member(user, Gitlab::Access::OWNER) + group3.add_member(user, Gitlab::Access::DEVELOPER) request_access_token! @payload = JSON::JWT.decode(json_response['id_token'], :skip_verification) diff --git a/spec/requests/projects/environments_controller_spec.rb b/spec/requests/projects/environments_controller_spec.rb index 5cdf507abef..0890b0c45da 100644 --- a/spec/requests/projects/environments_controller_spec.rb +++ b/spec/requests/projects/environments_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Projects::EnvironmentsController do - let_it_be(:project) { create(:project, :repository) } + let_it_be_with_refind(:project) { create(:project, :repository) } let(:environment) { create(:environment, name: 'production', project: project) } diff --git a/spec/serializers/member_user_entity_spec.rb b/spec/serializers/member_user_entity_spec.rb index 85f29845d65..4dd6848c47b 100644 --- a/spec/serializers/member_user_entity_spec.rb +++ b/spec/serializers/member_user_entity_spec.rb @@ -51,7 +51,7 @@ RSpec.describe MemberUserEntity do shared_examples 'correctly exposes user two_factor_enabled' do context 'when the current_user has a role lower than minimum manage member role' do before do - source.add_user(current_user, Gitlab::Access::DEVELOPER) + source.add_member(current_user, Gitlab::Access::DEVELOPER) end it 'does not expose user two_factor_enabled' do @@ -65,7 +65,7 @@ RSpec.describe MemberUserEntity do context 'when the current user has a minimum manage member role or higher' do before do - source.add_user(current_user, minimum_manage_member_role) + source.add_member(current_user, minimum_manage_member_role) end it 'matches json schema' do diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 628943e40ff..57a151efda6 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Groups::DestroyService do let(:remove_path) { group.path + "+#{group.id}+deleted" } before do - group.add_user(user, Gitlab::Access::OWNER) + group.add_member(user, Gitlab::Access::OWNER) end def destroy_group(group, user, async) @@ -168,8 +168,8 @@ RSpec.describe Groups::DestroyService do let(:group2_user) { create(:user) } before do - group1.add_user(group1_user, Gitlab::Access::OWNER) - group2.add_user(group2_user, Gitlab::Access::OWNER) + group1.add_member(group1_user, Gitlab::Access::OWNER) + group2.add_member(group2_user, Gitlab::Access::OWNER) end context 'when a project is shared with a group' do @@ -203,7 +203,7 @@ RSpec.describe Groups::DestroyService do let(:group3_user) { create(:user) } before do - group3.add_user(group3_user, Gitlab::Access::OWNER) + group3.add_member(group3_user, Gitlab::Access::OWNER) create(:group_group_link, shared_group: group2, shared_with_group: group3) group3.refresh_members_authorized_projects @@ -290,7 +290,7 @@ RSpec.describe Groups::DestroyService do let!(:shared_with_group_user) { create(:user) } before do - shared_with_group.add_user(shared_with_group_user, Gitlab::Access::MAINTAINER) + shared_with_group.add_member(shared_with_group_user, Gitlab::Access::MAINTAINER) create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group) shared_with_group.refresh_members_authorized_projects diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index 46c5e2a9818..c0e1691fe26 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -58,7 +58,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } before do - public_group.add_user(user, Gitlab::Access::OWNER) + public_group.add_member(user, Gitlab::Access::OWNER) create(:project, :public, group: public_group) expect(TodosDestroyer::GroupPrivateWorker).not_to receive(:perform_in) @@ -119,7 +119,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } before do - internal_group.add_user(user, Gitlab::Access::OWNER) + internal_group.add_member(user, Gitlab::Access::OWNER) create(:project, :internal, group: internal_group) expect(TodosDestroyer::GroupPrivateWorker).not_to receive(:perform_in) @@ -135,7 +135,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } before do - internal_group.add_user(user, Gitlab::Access::OWNER) + internal_group.add_member(user, Gitlab::Access::OWNER) create(:project, :private, group: internal_group) expect(TodosDestroyer::GroupPrivateWorker).to receive(:perform_in) @@ -233,7 +233,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(internal_group, user, visibility_level: 99) } before do - internal_group.add_user(user, Gitlab::Access::MAINTAINER) + internal_group.add_member(user, Gitlab::Access::MAINTAINER) end it "does not change permission level" do @@ -246,7 +246,7 @@ RSpec.describe Groups::UpdateService do let(:service) { described_class.new(internal_group, user, emails_disabled: true) } it 'updates the attribute' do - internal_group.add_user(user, Gitlab::Access::OWNER) + internal_group.add_member(user, Gitlab::Access::OWNER) expect { service.execute }.to change { internal_group.emails_disabled }.to(true) end @@ -280,7 +280,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(internal_group, user, path: SecureRandom.hex) } before do - internal_group.add_user(user, Gitlab::Access::MAINTAINER) + internal_group.add_member(user, Gitlab::Access::MAINTAINER) create(:project, :internal, group: internal_group) end diff --git a/spec/services/issues/related_branches_service_spec.rb b/spec/services/issues/related_branches_service_spec.rb index 53474c61907..7a4bae7f852 100644 --- a/spec/services/issues/related_branches_service_spec.rb +++ b/spec/services/issues/related_branches_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Issues::RelatedBranchesService, :clean_gitlab_redis_cache do +RSpec.describe Issues::RelatedBranchesService do let_it_be(:developer) { create(:user) } let_it_be(:issue) { create(:issue) } diff --git a/spec/services/members/creator_service_spec.rb b/spec/services/members/creator_service_spec.rb index 8b1df2ab86d..ad4c649086b 100644 --- a/spec/services/members/creator_service_spec.rb +++ b/spec/services/members/creator_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Members::CreatorService do describe '#execute' do it 'raises error for new member on authorization check implementation' do expect do - described_class.add_user(source, user, :maintainer, current_user: current_user) + described_class.add_member(source, user, :maintainer, current_user: current_user) end.to raise_error(NotImplementedError) end @@ -19,7 +19,7 @@ RSpec.describe Members::CreatorService do source.add_developer(user) expect do - described_class.add_user(source, user, :maintainer, current_user: current_user) + described_class.add_member(source, user, :maintainer, current_user: current_user) end.to raise_error(NotImplementedError) end end diff --git a/spec/services/members/groups/creator_service_spec.rb b/spec/services/members/groups/creator_service_spec.rb index b80b7998eac..4130fbd44fa 100644 --- a/spec/services/members/groups/creator_service_spec.rb +++ b/spec/services/members/groups/creator_service_spec.rb @@ -14,13 +14,13 @@ RSpec.describe Members::Groups::CreatorService do it_behaves_like 'owner management' - describe '.add_users' do + describe '.add_members' do it_behaves_like 'bulk member creation' do let_it_be(:member_type) { GroupMember } end end - describe '.add_user' do + describe '.add_member' do it_behaves_like 'member creation' do let_it_be(:member_type) { GroupMember } end @@ -30,7 +30,7 @@ RSpec.describe Members::Groups::CreatorService do expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait).once 1.upto(3) do - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) end end end diff --git a/spec/services/members/projects/creator_service_spec.rb b/spec/services/members/projects/creator_service_spec.rb index 38955122ab0..8304bee2ffc 100644 --- a/spec/services/members/projects/creator_service_spec.rb +++ b/spec/services/members/projects/creator_service_spec.rb @@ -14,13 +14,13 @@ RSpec.describe Members::Projects::CreatorService do it_behaves_like 'owner management' - describe '.add_users' do + describe '.add_members' do it_behaves_like 'bulk member creation' do let_it_be(:member_type) { ProjectMember } end end - describe '.add_user' do + describe '.add_member' do it_behaves_like 'member creation' do let_it_be(:member_type) { ProjectMember } end @@ -30,7 +30,7 @@ RSpec.describe Members::Projects::CreatorService do expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to receive(:bulk_perform_in).once 1.upto(3) do - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 8e0f964d965..98fe8a40c61 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -582,8 +582,8 @@ RSpec.describe NotificationService, :mailer do before do note.project.namespace_id = group.id - group.add_user(@u_watcher, GroupMember::MAINTAINER) - group.add_user(@u_custom_global, GroupMember::MAINTAINER) + group.add_member(@u_watcher, GroupMember::MAINTAINER) + group.add_member(@u_custom_global, GroupMember::MAINTAINER) note.project.save! @u_watcher.notification_settings_for(note.project).participating! @@ -3850,7 +3850,7 @@ RSpec.describe NotificationService, :mailer do # Group member: global=watch, group=global @g_global_watcher ||= create_global_setting_for(create(:user), :watch) - group.add_users([@g_watcher, @g_global_watcher], :maintainer) + group.add_members([@g_watcher, @g_global_watcher], :maintainer) group end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 278ca148b74..7361b9b8e35 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -224,7 +224,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi context 'when flushing caches fail due to Redis' do before do new_user = create(:user) - project.team.add_user(new_user, Gitlab::Access::DEVELOPER) + project.team.add_member(new_user, Gitlab::Access::DEVELOPER) allow_any_instance_of(described_class).to receive(:flush_caches).and_raise(::Redis::CannotConnectError) end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index e292c13c46e..b1b45e89c1b 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -32,14 +32,14 @@ RSpec.describe Projects::ForkService do external_authorization_classification_label: 'classification-label') @to_user = create(:user) @to_namespace = @to_user.namespace - @from_project.add_user(@to_user, :developer) + @from_project.add_member(@to_user, :developer) end context 'fork project' do context 'when forker is a guest' do before do @guest = create(:user) - @from_project.add_user(@guest, :guest) + @from_project.add_member(@guest, :guest) end subject { fork_project(@from_project, @guest, using_service: true) } @@ -261,10 +261,10 @@ RSpec.describe Projects::ForkService do description: 'Wow, such a cool project!', ci_config_path: 'debian/salsa-ci.yml') @group = create(:group) - @group.add_user(@group_owner, GroupMember::OWNER) - @group.add_user(@developer, GroupMember::DEVELOPER) - @project.add_user(@developer, :developer) - @project.add_user(@group_owner, :developer) + @group.add_member(@group_owner, GroupMember::OWNER) + @group.add_member(@developer, GroupMember::DEVELOPER) + @project.add_member(@developer, :developer) + @project.add_member(@group_owner, :developer) @opts = { namespace: @group, using_service: true } end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 1241491d914..f019434a4fe 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -424,7 +424,7 @@ RSpec.describe Projects::UpdateService do it 'does not update when not project owner' do maintainer = create(:user) - project.add_user(maintainer, :maintainer) + project.add_member(maintainer, :maintainer) expect { update_project(project, maintainer, emails_disabled: true) } .not_to change { project.emails_disabled } diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index f7ed6006099..3f11eaa7e93 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe QuickActions::InterpretService do + include AfterNextHelpers + let_it_be(:group) { create(:group, :crm_enabled) } let_it_be(:public_project) { create(:project, :public, group: group) } let_it_be(:repository_project) { create(:project, :repository) } @@ -17,8 +19,9 @@ RSpec.describe QuickActions::InterpretService do let(:milestone) { create(:milestone, project: project, title: '9.10') } let(:commit) { create(:commit, project: project) } + let(:current_user) { developer } - subject(:service) { described_class.new(project, developer) } + subject(:service) { described_class.new(project, current_user) } before_all do public_project.add_developer(developer) @@ -682,6 +685,58 @@ RSpec.describe QuickActions::InterpretService do end shared_examples 'assign command' do + it 'assigns to me' do + cmd = '/assign me' + + _, updates, _ = service.execute(cmd, issuable) + + expect(updates).to eq(assignee_ids: [current_user.id]) + end + + it 'does not assign to group members' do + grp = create(:group) + grp.add_developer(developer) + grp.add_developer(developer2) + + cmd = "/assign #{grp.to_reference}" + + _, updates, message = service.execute(cmd, issuable) + + expect(updates).to be_blank + expect(message).to include('Failed to find users') + end + + context 'when there are too many references' do + before do + stub_const('Gitlab::QuickActions::UsersExtractor::MAX_QUICK_ACTION_USERS', 2) + end + + it 'says what went wrong' do + cmd = '/assign her and you, me and them' + + _, updates, message = service.execute(cmd, issuable) + + expect(updates).to be_blank + expect(message).to include('Too many references. Quick actions are limited to at most 2 user references') + end + end + + context 'when the user extractor raises an uninticipated error' do + before do + allow_next(Gitlab::QuickActions::UsersExtractor) + .to receive(:execute).and_raise(Gitlab::QuickActions::UsersExtractor::Error) + end + + it 'tracks the exception in dev, and reports a generic message in production' do + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).twice + + _, updates, message = service.execute('/assign some text', issuable) + + expect(updates).to be_blank + expect(message).to include('Something went wrong') + end + end + it 'assigns to users with escaped underscores' do user = create(:user) base = user.username diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index e4582e19416..1cb44366457 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -186,8 +186,8 @@ RSpec.describe TodoService do before do group.add_owner(author) - group.add_user(member, Gitlab::Access::DEVELOPER) - group.add_user(john_doe, Gitlab::Access::DEVELOPER) + group.add_member(member, Gitlab::Access::DEVELOPER) + group.add_member(john_doe, Gitlab::Access::DEVELOPER) service.new_issue(issue, author) end diff --git a/spec/support/helpers/project_helpers.rb b/spec/support/helpers/project_helpers.rb index ef8947ab340..2427ed2bcc9 100644 --- a/spec/support/helpers/project_helpers.rb +++ b/spec/support/helpers/project_helpers.rb @@ -13,7 +13,7 @@ module ProjectHelpers when :admin create(:user, :admin, name: 'admin') else - create(:user, name: membership).tap { |u| target.add_user(u, membership) } + create(:user, name: membership).tap { |u| target.add_member(u, membership) } end end diff --git a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb index 622a88e8323..f3c616c7d9b 100644 --- a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb +++ b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb @@ -962,7 +962,7 @@ RSpec.shared_examples 'issues or work items finder' do |factory, execute_context group = create(:group) project = create(:project, group: group) item = create(factory, project: project) - group.add_user(user, :owner) + group.add_member(user, :owner) expect(items).to include(item) end diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb index 75fff11cecd..9ac173341d7 100644 --- a/spec/support/shared_examples/models/member_shared_examples.rb +++ b/spec/support/shared_examples/models/member_shared_examples.rb @@ -80,7 +80,7 @@ RSpec.shared_examples_for "member creation" do let_it_be(:admin) { create(:admin) } it 'returns a Member object', :aggregate_failures do - member = described_class.add_user(source, user, :maintainer) + member = described_class.add_member(source, user, :maintainer) expect(member).to be_a member_type expect(member).to be_persisted @@ -99,7 +99,7 @@ RSpec.shared_examples_for "member creation" do end it 'does not update the member' do - member = described_class.add_user(source, project_bot, :maintainer, current_user: user) + member = described_class.add_member(source, project_bot, :maintainer, current_user: user) expect(source.users.reload).to include(project_bot) expect(member).to be_persisted @@ -110,7 +110,7 @@ RSpec.shared_examples_for "member creation" do context 'when project_bot is not already a member' do it 'adds the member' do - member = described_class.add_user(source, project_bot, :maintainer, current_user: user) + member = described_class.add_member(source, project_bot, :maintainer, current_user: user) expect(source.users.reload).to include(project_bot) expect(member).to be_persisted @@ -120,7 +120,7 @@ RSpec.shared_examples_for "member creation" do context 'when admin mode is enabled', :enable_admin_mode, :aggregate_failures do it 'sets members.created_by to the given admin current_user' do - member = described_class.add_user(source, user, :maintainer, current_user: admin) + member = described_class.add_member(source, user, :maintainer, current_user: admin) expect(member).to be_persisted expect(source.users.reload).to include(user) @@ -130,7 +130,7 @@ RSpec.shared_examples_for "member creation" do context 'when admin mode is disabled' do it 'rejects setting members.created_by to the given admin current_user', :aggregate_failures do - member = described_class.add_user(source, user, :maintainer, current_user: admin) + member = described_class.add_member(source, user, :maintainer, current_user: admin) expect(member).not_to be_persisted expect(source.users.reload).not_to include(user) @@ -139,7 +139,7 @@ RSpec.shared_examples_for "member creation" do end it 'sets members.expires_at to the given expires_at' do - member = described_class.add_user(source, user, :maintainer, expires_at: Date.new(2016, 9, 22)) + member = described_class.add_member(source, user, :maintainer, expires_at: Date.new(2016, 9, 22)) expect(member.expires_at).to eq(Date.new(2016, 9, 22)) end @@ -148,7 +148,7 @@ RSpec.shared_examples_for "member creation" do it "accepts the :#{sym_key} symbol as access level", :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.add_user(source, user.id, sym_key) + member = described_class.add_member(source, user.id, sym_key) expect(member.access_level).to eq(int_access_level) expect(source.users.reload).to include(user) @@ -157,7 +157,7 @@ RSpec.shared_examples_for "member creation" do it "accepts the #{int_access_level} integer as access level", :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.add_user(source, user.id, int_access_level) + member = described_class.add_member(source, user.id, int_access_level) expect(member.access_level).to eq(int_access_level) expect(source.users.reload).to include(user) @@ -169,7 +169,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, user.id, :maintainer) + described_class.add_member(source, user.id, :maintainer) expect(source.users.reload).to include(user) end @@ -179,7 +179,7 @@ RSpec.shared_examples_for "member creation" do it 'does not add the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, non_existing_record_id, :maintainer) + described_class.add_member(source, non_existing_record_id, :maintainer) expect(source.users.reload).not_to include(user) end @@ -189,7 +189,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) expect(source.users.reload).to include(user) end @@ -205,7 +205,7 @@ RSpec.shared_examples_for "member creation" do expect(source.requesters.exists?(user_id: user)).to be_truthy expect do - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) end.to raise_error(Gitlab::Access::AccessDeniedError) expect(source.users.reload).not_to include(user) @@ -217,7 +217,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, user.email, :maintainer) + described_class.add_member(source, user.email, :maintainer) expect(source.users.reload).to include(user) end @@ -227,7 +227,7 @@ RSpec.shared_examples_for "member creation" do it 'creates an invited member' do expect(source.users).not_to include(user) - described_class.add_user(source, 'user@example.com', :maintainer) + described_class.add_member(source, 'user@example.com', :maintainer) expect(source.members.invite.pluck(:invite_email)).to include('user@example.com') end @@ -237,7 +237,7 @@ RSpec.shared_examples_for "member creation" do it 'creates an invited member', :aggregate_failures do email_starting_with_number = "#{user.id}_email@example.com" - described_class.add_user(source, email_starting_with_number, :maintainer) + described_class.add_member(source, email_starting_with_number, :maintainer) expect(source.members.invite.pluck(:invite_email)).to include(email_starting_with_number) expect(source.users.reload).not_to include(user) @@ -249,7 +249,7 @@ RSpec.shared_examples_for "member creation" do it 'creates the member' do expect(source.users).not_to include(user) - described_class.add_user(source, user, :maintainer, current_user: admin) + described_class.add_member(source, user, :maintainer, current_user: admin) expect(source.users.reload).to include(user) end @@ -263,7 +263,7 @@ RSpec.shared_examples_for "member creation" do expect(source.users).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy - described_class.add_user(source, user, :maintainer, current_user: admin) + described_class.add_member(source, user, :maintainer, current_user: admin) expect(source.users.reload).to include(user) expect(source.requesters.reload.exists?(user_id: user)).to be_falsy @@ -275,7 +275,7 @@ RSpec.shared_examples_for "member creation" do it 'does not create the member', :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.add_user(source, user, :maintainer, current_user: user) + member = described_class.add_member(source, user, :maintainer, current_user: user) expect(source.users.reload).not_to include(user) expect(member).not_to be_persisted @@ -290,7 +290,7 @@ RSpec.shared_examples_for "member creation" do expect(source.users).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy - described_class.add_user(source, user, :maintainer, current_user: user) + described_class.add_member(source, user, :maintainer, current_user: user) expect(source.users.reload).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy @@ -300,14 +300,14 @@ RSpec.shared_examples_for "member creation" do context 'when member already exists' do before do - source.add_user(user, :developer) + source.add_member(user, :developer) end context 'with no current_user' do it 'updates the member' do expect(source.users).to include(user) - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) end @@ -317,7 +317,7 @@ RSpec.shared_examples_for "member creation" do it 'updates the member' do expect(source.users).to include(user) - described_class.add_user(source, user, :maintainer, current_user: admin) + described_class.add_member(source, user, :maintainer, current_user: admin) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) end @@ -327,7 +327,7 @@ RSpec.shared_examples_for "member creation" do it 'does not update the member' do expect(source.users).to include(user) - described_class.add_user(source, user, :maintainer, current_user: user) + described_class.add_member(source, user, :maintainer, current_user: user) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::DEVELOPER) end @@ -345,12 +345,12 @@ RSpec.shared_examples_for "bulk member creation" do # maintainers cannot add owners source.add_maintainer(user) - expect(described_class.add_users(source, [user1, user2], :owner, current_user: user)).to be_empty + expect(described_class.add_members(source, [user1, user2], :owner, current_user: user)).to be_empty end end it 'returns Member objects' do - members = described_class.add_users(source, [user1, user2], :maintainer) + members = described_class.add_members(source, [user1, user2], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2) expect(members).to all(be_a(member_type)) @@ -358,7 +358,7 @@ RSpec.shared_examples_for "bulk member creation" do end it 'returns an empty array' do - members = described_class.add_users(source, [], :maintainer) + members = described_class.add_members(source, [], :maintainer) expect(members).to be_a Array expect(members).to be_empty @@ -367,7 +367,7 @@ RSpec.shared_examples_for "bulk member creation" do it 'supports different formats' do list = ['joe@local.test', admin, user1.id, user2.id.to_s] - members = described_class.add_users(source, list, :maintainer) + members = described_class.add_members(source, list, :maintainer) expect(members.size).to eq(4) expect(members.first).to be_invite @@ -375,7 +375,7 @@ RSpec.shared_examples_for "bulk member creation" do context 'with de-duplication' do it 'has the same user by id and user' do - members = described_class.add_users(source, [user1.id, user1, user1.id, user2, user2.id, user2], :maintainer) + members = described_class.add_members(source, [user1.id, user1, user1.id, user2, user2.id, user2], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2) expect(members).to all(be_a(member_type)) @@ -383,7 +383,7 @@ RSpec.shared_examples_for "bulk member creation" do end it 'has the same user sent more than once' do - members = described_class.add_users(source, [user1, user1], :maintainer) + members = described_class.add_members(source, [user1, user1], :maintainer) expect(members.map(&:user)).to contain_exactly(user1) expect(members).to all(be_a(member_type)) @@ -392,7 +392,7 @@ RSpec.shared_examples_for "bulk member creation" do end it 'with the same user sent more than once by user and by email' do - members = described_class.add_users(source, [user1, user1.email], :maintainer) + members = described_class.add_members(source, [user1, user1.email], :maintainer) expect(members.map(&:user)).to contain_exactly(user1) expect(members).to all(be_a(member_type)) @@ -400,7 +400,7 @@ RSpec.shared_examples_for "bulk member creation" do end it 'with the same user sent more than once by user id and by email' do - members = described_class.add_users(source, [user1.id, user1.email], :maintainer) + members = described_class.add_members(source, [user1.id, user1.email], :maintainer) expect(members.map(&:user)).to contain_exactly(user1) expect(members).to all(be_a(member_type)) @@ -409,12 +409,12 @@ RSpec.shared_examples_for "bulk member creation" do context 'when a member already exists' do before do - source.add_user(user1, :developer) + source.add_member(user1, :developer) end it 'has the same user sent more than once with the member already existing' do expect do - members = described_class.add_users(source, [user1, user1, user2], :maintainer) + members = described_class.add_members(source, [user1, user1, user2], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) @@ -425,7 +425,7 @@ RSpec.shared_examples_for "bulk member creation" do user3 = create(:user) expect do - members = described_class.add_users(source, [user1.id, user2, user3.id], :maintainer) + members = described_class.add_members(source, [user1.id, user2, user3.id], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2, user3) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) @@ -436,7 +436,7 @@ RSpec.shared_examples_for "bulk member creation" do user3 = create(:user) expect do - members = described_class.add_users(source, [user1, user2, user3], :maintainer) + members = described_class.add_members(source, [user1, user2, user3], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2, user3) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) @@ -448,7 +448,7 @@ RSpec.shared_examples_for "bulk member creation" do let(:task_project) { source.is_a?(Group) ? create(:project, group: source) : source } it 'creates a member_task with the correct attributes', :aggregate_failures do - members = described_class.add_users(source, [user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: task_project.id) + members = described_class.add_members(source, [user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: task_project.id) member = members.last expect(member.tasks_to_be_done).to match_array([:ci, :code]) @@ -457,7 +457,7 @@ RSpec.shared_examples_for "bulk member creation" do context 'with an already existing member' do before do - source.add_user(user1, :developer) + source.add_member(user1, :developer) end it 'does not update tasks to be done if tasks already exist', :aggregate_failures do @@ -465,7 +465,7 @@ RSpec.shared_examples_for "bulk member creation" do create(:member_task, member: member, project: task_project, tasks_to_be_done: %w(code ci)) expect do - described_class.add_users(source, + described_class.add_members(source, [user1.id], :developer, tasks_to_be_done: %w(issues), @@ -479,7 +479,7 @@ RSpec.shared_examples_for "bulk member creation" do it 'adds tasks to be done if they do not exist', :aggregate_failures do expect do - described_class.add_users(source, + described_class.add_members(source, [user1.id], :developer, tasks_to_be_done: %w(issues), diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb index d7c2b645de9..bb2f8965294 100644 --- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb @@ -72,7 +72,7 @@ RSpec.shared_examples 'conan search endpoint' do project.update!(visibility: 'private') project.team.truncate user.project_authorizations.delete_all - project.add_user(user, role) unless role == :anonymous + project.add_member(user, role) unless role == :anonymous get api(url), params: params, headers: headers end diff --git a/spec/views/shared/deploy_tokens/_form.html.haml_spec.rb b/spec/views/shared/deploy_tokens/_form.html.haml_spec.rb index 5ac42952f78..74ad0ccb77a 100644 --- a/spec/views/shared/deploy_tokens/_form.html.haml_spec.rb +++ b/spec/views/shared/deploy_tokens/_form.html.haml_spec.rb @@ -10,7 +10,7 @@ RSpec.describe 'shared/deploy_tokens/_form.html.haml' do RSpec.shared_examples "display deploy token settings" do |role, shows_package_registry_permissions| before do - subject.add_user(user, role) + subject.add_member(user, role) allow(view).to receive(:current_user).and_return(user) stub_config(packages: { enabled: packages_enabled }) end diff --git a/spec/workers/disallow_two_factor_for_group_worker_spec.rb b/spec/workers/disallow_two_factor_for_group_worker_spec.rb index a69dd893f81..f30b12dd7f4 100644 --- a/spec/workers/disallow_two_factor_for_group_worker_spec.rb +++ b/spec/workers/disallow_two_factor_for_group_worker_spec.rb @@ -13,7 +13,7 @@ RSpec.describe DisallowTwoFactorForGroupWorker do end it "updates group members" do - group.add_user(user, GroupMember::DEVELOPER) + group.add_member(user, GroupMember::DEVELOPER) described_class.new.perform(group.id) diff --git a/spec/workers/remove_expired_members_worker_spec.rb b/spec/workers/remove_expired_members_worker_spec.rb index 6d0d4aeef89..8d7d488094f 100644 --- a/spec/workers/remove_expired_members_worker_spec.rb +++ b/spec/workers/remove_expired_members_worker_spec.rb @@ -47,7 +47,7 @@ RSpec.describe RemoveExpiredMembersWorker do let_it_be(:expired_project_bot) { create(:user, :project_bot) } before do - project.add_user(expired_project_bot, :maintainer, expires_at: 1.day.from_now) + project.add_member(expired_project_bot, :maintainer, expires_at: 1.day.from_now) travel_to(3.days.from_now) end @@ -67,7 +67,7 @@ RSpec.describe RemoveExpiredMembersWorker do let_it_be(:other_project_bot) { create(:user, :project_bot) } before do - project.add_user(other_project_bot, :maintainer, expires_at: 10.days.from_now) + project.add_member(other_project_bot, :maintainer, expires_at: 10.days.from_now) travel_to(3.days.from_now) end diff --git a/workhorse/go.mod b/workhorse/go.mod index 15279b50529..4b0478ffa68 100644 --- a/workhorse/go.mod +++ b/workhorse/go.mod @@ -6,7 +6,7 @@ require ( github.com/Azure/azure-storage-blob-go v0.14.0 github.com/BurntSushi/toml v0.3.1 github.com/FZambia/sentinel v1.1.0 - github.com/alecthomas/chroma v0.10.0 + github.com/alecthomas/chroma/v2 v2.2.0 github.com/aws/aws-sdk-go v1.43.31 github.com/disintegration/imaging v1.6.2 github.com/getsentry/raven-go v0.2.0 diff --git a/workhorse/go.sum b/workhorse/go.sum index 51c7a51c002..c3367098416 100644 --- a/workhorse/go.sum +++ b/workhorse/go.sum @@ -182,8 +182,10 @@ github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia github.com/ajg/form v1.5.1/go.mod h1:uL1WgH+h2mgNtvBq0339dVnzXdBETtL2LeUXaIv25UY= github.com/ajstarks/svgo v0.0.0-20180226025133-644b8db467af/go.mod h1:K08gAheRH3/J6wwsYMMT4xOr94bZjxIelGM0+d/wbFw= github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7/go.mod h1:6zEj6s6u/ghQa61ZWa/C2Aw3RkjiTBOix7dkqa1VLIs= -github.com/alecthomas/chroma v0.10.0 h1:7XDcGkCQopCNKjZHfYrNLraA+M7e0fMiJ/Mfikbfjek= -github.com/alecthomas/chroma v0.10.0/go.mod h1:jtJATyUxlIORhUOFNA9NZDWGAQ8wpxQQqNSB4rjA/1s= +github.com/alecthomas/chroma/v2 v2.2.0 h1:Aten8jfQwUqEdadVFFjNyjx7HTexhKP0XuqBG67mRDY= +github.com/alecthomas/chroma/v2 v2.2.0/go.mod h1:vf4zrexSH54oEjJ7EdB65tGNHmH3pGZmVkgTP5RHvAs= +github.com/alecthomas/repr v0.0.0-20220113201626-b1b626ac65ae h1:zzGwJfFlFGD94CyyYwCJeSuD32Gj9GTaSi5y9hoVzdY= +github.com/alecthomas/repr v0.0.0-20220113201626-b1b626ac65ae/go.mod h1:2kn6fqh/zIyPLmm3ugklbEi5hg5wS435eygvNfaDQL8= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= diff --git a/workhorse/internal/lsif_transformer/parser/code_hover.go b/workhorse/internal/lsif_transformer/parser/code_hover.go index 5651ea8e5a3..25550cce29e 100644 --- a/workhorse/internal/lsif_transformer/parser/code_hover.go +++ b/workhorse/internal/lsif_transformer/parser/code_hover.go @@ -5,8 +5,8 @@ import ( "strings" "unicode/utf8" - "github.com/alecthomas/chroma" - "github.com/alecthomas/chroma/lexers" + "github.com/alecthomas/chroma/v2" + "github.com/alecthomas/chroma/v2/lexers" ) const maxValueSize = 250