From ad969fee0a4ee6c362cb367e1906b67f6cb22b37 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 1 May 2023 12:11:38 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-10-stable-ee --- lib/gitlab/checks/branch_check.rb | 2 +- spec/lib/gitlab/checks/branch_check_spec.rb | 8 +++- ..._max_access_level_in_projects_preloader_spec.rb | 3 +- workhorse/internal/headers/content_headers.go | 32 +++++++++++-- workhorse/internal/headers/content_headers_test.go | 56 ++++++++++++++++++++++ .../contentprocessor/contentprocessor_test.go | 4 +- workhorse/testdata/index.xhtml | 9 ++++ workhorse/testdata/test.xml | 6 +++ workhorse/testdata/xml.svg | 7 +++ 9 files changed, 116 insertions(+), 11 deletions(-) create mode 100644 workhorse/internal/headers/content_headers_test.go create mode 100644 workhorse/testdata/index.xhtml create mode 100644 workhorse/testdata/test.xml create mode 100644 workhorse/testdata/xml.svg diff --git a/lib/gitlab/checks/branch_check.rb b/lib/gitlab/checks/branch_check.rb index e8f13a92ee7..fa7c4972c91 100644 --- a/lib/gitlab/checks/branch_check.rb +++ b/lib/gitlab/checks/branch_check.rb @@ -42,7 +42,7 @@ module Gitlab def prohibited_branch_checks return if deletion? - if branch_name =~ /\A\h{40}\z/ + if branch_name =~ %r{\A\h{40}(/|\z)} raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_hex_branch_name] end end diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb index d6280d3c28c..7f535e86d69 100644 --- a/spec/lib/gitlab/checks/branch_check_spec.rb +++ b/spec/lib/gitlab/checks/branch_check_spec.rb @@ -26,8 +26,14 @@ RSpec.describe Gitlab::Checks::BranchCheck do expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a branch with a 40-character hexadecimal branch name.") end + it "prohibits 40-character hexadecimal branch names as the start of a path" do + allow(subject).to receive(:branch_name).and_return("267208abfe40e546f5e847444276f7d43a39503e/test") + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a branch with a 40-character hexadecimal branch name.") + end + it "doesn't prohibit a nested hexadecimal in a branch name" do - allow(subject).to receive(:branch_name).and_return("fix-267208abfe40e546f5e847444276f7d43a39503e") + allow(subject).to receive(:branch_name).and_return("267208abfe40e546f5e847444276f7d43a39503e-fix") expect { subject.validate! }.not_to raise_error end diff --git a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb index de10653d87e..a2ab59f56ab 100644 --- a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb @@ -23,8 +23,7 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do # we have an existing N+1, one for each project for which user is not a member # in this spec, project_3, project_4, project_5 # https://gitlab.com/gitlab-org/gitlab/-/issues/362890 - ee_only_policy_check_queries = Gitlab.ee? ? 1 : 0 - expect { query }.to make_queries(projects.size + 3 + ee_only_policy_check_queries) + expect { query }.to make_queries(projects.size + 3) end end diff --git a/workhorse/internal/headers/content_headers.go b/workhorse/internal/headers/content_headers.go index 854cc8abddd..54c7c1bdd95 100644 --- a/workhorse/internal/headers/content_headers.go +++ b/workhorse/internal/headers/content_headers.go @@ -1,6 +1,7 @@ package headers import ( + "mime" "net/http" "regexp" @@ -13,8 +14,9 @@ var ( imageTypeRegex = regexp.MustCompile(`^image/*`) svgMimeTypeRegex = regexp.MustCompile(`^image/svg\+xml$`) - textTypeRegex = regexp.MustCompile(`^text/*`) - + textTypeRegex = regexp.MustCompile(`^text/*`) + xmlTypeRegex = regexp.MustCompile(`^text/xml`) + xhtmlTypeRegex = regexp.MustCompile(`^text/html`) videoTypeRegex = regexp.MustCompile(`^video/*`) pdfTypeRegex = regexp.MustCompile(`application\/pdf`) @@ -26,6 +28,8 @@ var ( // Mime types that can't be inlined. Usually subtypes of main types var forbiddenInlineTypes = []*regexp.Regexp{svgMimeTypeRegex} +var htmlRenderingTypes = []*regexp.Regexp{xmlTypeRegex, xhtmlTypeRegex} + // Mime types that can be inlined. We can add global types like "image/" or // specific types like "text/plain". If there is a specific type inside a global // allowed type that can't be inlined we must add it to the forbiddenInlineTypes var. @@ -38,12 +42,28 @@ const ( textPlainContentType = "text/plain; charset=utf-8" attachmentDispositionText = "attachment" inlineDispositionText = "inline" + dummyFilename = "blob" ) func SafeContentHeaders(data []byte, contentDisposition string) (string, string) { - contentType := safeContentType(data) + detectedContentType := detectContentType(data) + + contentType := safeContentType(detectedContentType) contentDisposition = safeContentDisposition(contentType, contentDisposition) + // Some browsers will render XML inline unless a filename directive is provided with a non-xml file extension + // This overrides the filename directive in the case of XML data + for _, element := range htmlRenderingTypes { + if isType(detectedContentType, element) { + disposition, directives, err := mime.ParseMediaType(contentDisposition) + if err == nil { + directives["filename"] = dummyFilename + contentDisposition = mime.FormatMediaType(disposition, directives) + break + } + } + } + // Set attachments to application/octet-stream since browsers can do // a better job distinguishing certain types (for example: ZIP files // vs. Microsoft .docx files). However, browsers may safely render SVGs even @@ -56,15 +76,17 @@ func SafeContentHeaders(data []byte, contentDisposition string) (string, string) return contentType, contentDisposition } -func safeContentType(data []byte) string { +func detectContentType(data []byte) string { // Special case for svg because DetectContentType detects it as text if svg.Is(data) { return svgContentType } // Override any existing Content-Type header from other ResponseWriters - contentType := http.DetectContentType(data) + return http.DetectContentType(data) +} +func safeContentType(contentType string) string { // http.DetectContentType does not support JavaScript and would only // return text/plain. But for cautionary measures, just in case they start supporting // it down the road and start returning application/javascript, we want to handle it now diff --git a/workhorse/internal/headers/content_headers_test.go b/workhorse/internal/headers/content_headers_test.go new file mode 100644 index 00000000000..7cfce335d88 --- /dev/null +++ b/workhorse/internal/headers/content_headers_test.go @@ -0,0 +1,56 @@ +package headers + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func fileContents(fileName string) []byte { + fileContents, _ := os.ReadFile(fileName) + return fileContents +} + +func TestHeaders(t *testing.T) { + tests := []struct { + desc string + fileContents []byte + expectedContentType string + expectedContentDisposition string + }{ + { + desc: "XML file", + fileContents: fileContents("../../testdata/test.xml"), + expectedContentType: "text/plain; charset=utf-8", + expectedContentDisposition: "inline; filename=blob", + }, + { + desc: "XHTML file", + fileContents: fileContents("../../testdata/index.xhtml"), + expectedContentType: "text/plain; charset=utf-8", + expectedContentDisposition: "inline; filename=blob", + }, + { + desc: "svg+xml file", + fileContents: fileContents("../../testdata/xml.svg"), + expectedContentType: "image/svg+xml", + expectedContentDisposition: "attachment", + }, + { + desc: "text file", + fileContents: []byte(`a text file`), + expectedContentType: "text/plain; charset=utf-8", + expectedContentDisposition: "inline", + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + contentType, newContentDisposition := SafeContentHeaders(test.fileContents, "") + + require.Equal(t, test.expectedContentType, contentType) + require.Equal(t, test.expectedContentDisposition, newContentDisposition) + }) + } +} diff --git a/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go b/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go index b04263de6b9..e863935be6f 100644 --- a/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go +++ b/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go @@ -51,13 +51,13 @@ func TestSetProperContentTypeAndDisposition(t *testing.T) { { desc: "HTML type", contentType: "text/plain; charset=utf-8", - contentDisposition: "inline", + contentDisposition: "inline; filename=blob", body: "Hello world!", }, { desc: "Javascript within HTML type", contentType: "text/plain; charset=utf-8", - contentDisposition: "inline", + contentDisposition: "inline; filename=blob", body: "", }, { diff --git a/workhorse/testdata/index.xhtml b/workhorse/testdata/index.xhtml new file mode 100644 index 00000000000..1dd50a70e69 --- /dev/null +++ b/workhorse/testdata/index.xhtml @@ -0,0 +1,9 @@ + + + + Title of document + + + + diff --git a/workhorse/testdata/test.xml b/workhorse/testdata/test.xml new file mode 100644 index 00000000000..54b94e62355 --- /dev/null +++ b/workhorse/testdata/test.xml @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/workhorse/testdata/xml.svg b/workhorse/testdata/xml.svg new file mode 100644 index 00000000000..c41c4c44b49 --- /dev/null +++ b/workhorse/testdata/xml.svg @@ -0,0 +1,7 @@ + + + + + +
hello this is html
+ -- cgit v1.2.1 From 53005cb586fcec5d4e7737afb129ca7a86fb954b Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 1 May 2023 12:11:52 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-10-stable-ee --- doc/integration/saml.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/integration/saml.md b/doc/integration/saml.md index 888dd47a968..edf8d3d316c 100644 --- a/doc/integration/saml.md +++ b/doc/integration/saml.md @@ -3119,7 +3119,7 @@ such as the following: | Encrypt SAML assertion | Optional | Uses TLS between your identity provider, the user's browser, and GitLab. | | Sign SAML assertion | Optional | Validates the integrity of a SAML assertion. When active, signs the whole response. | | Check SAML request signature | Optional | Checks the signature on the SAML response. | -| Default RelayState | Optional | Specifies the URL users should end up on after successfully signing in through SAML at your IdP. | +| Default RelayState | Optional | Specifies the sub-paths of the base URL that users should end up on after successfully signing in through SAML at your IdP. | | NameID format | Persistent | See [NameID format details](../user/group/saml_sso/index.md#nameid-format). | | Additional URLs | Optional | May include the issuer, identifier, or assertion consumer service URL in other fields on some providers. | -- cgit v1.2.1 From 2655540094e856f3048fb737a19e4316d8264623 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 1 May 2023 12:13:04 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-10-stable-ee --- app/helpers/avatars_helper.rb | 2 +- lib/banzai/filter/asset_proxy_filter.rb | 44 +++++++++++++++++++++- spec/helpers/avatars_helper_spec.rb | 16 ++++++-- spec/lib/banzai/filter/asset_proxy_filter_spec.rb | 9 +++++ .../banzai/filter/commit_trailers_filter_spec.rb | 2 +- 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index 0fac2cb5fc5..57075a44d0f 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -116,7 +116,7 @@ module AvatarsHelper private def avatar_icon_by_user_email_or_gravatar(email, size, scale, only_path:) - user = User.find_by_any_email(email) + user = User.with_public_email(email).first if user avatar_icon_for_user(user, size, scale, only_path: only_path) diff --git a/lib/banzai/filter/asset_proxy_filter.rb b/lib/banzai/filter/asset_proxy_filter.rb index 4c14ee7299b..6371a8f23af 100644 --- a/lib/banzai/filter/asset_proxy_filter.rb +++ b/lib/banzai/filter/asset_proxy_filter.rb @@ -6,11 +6,35 @@ module Banzai # as well as hiding the customer's IP address when requesting images. # Copies the original img `src` to `data-canonical-src` then replaces the # `src` with a new url to the proxy server. - class AssetProxyFilter < HTML::Pipeline::CamoFilter + # + # Based on https://github.com/gjtorikian/html-pipeline/blob/v2.14.3/lib/html/pipeline/camo_filter.rb + class AssetProxyFilter < HTML::Pipeline::Filter def initialize(text, context = nil, result = nil) super end + def call + return doc unless asset_proxy_enabled? + + doc.search('img').each do |element| + original_src = element['src'] + next unless original_src + + begin + uri = URI.parse(original_src) + rescue StandardError + next + end + + next if uri.host.nil? && !original_src.start_with?('///') + next if asset_host_allowed?(uri.host) + + element['src'] = asset_proxy_url(original_src) + element['data-canonical-src'] = original_src + end + doc + end + def validate needs(:asset_proxy, :asset_proxy_secret_key) if asset_proxy_enabled? end @@ -63,6 +87,24 @@ module Banzai application_settings.try(:asset_proxy_whitelist).presence || [Gitlab.config.gitlab.host] end + + private + + def asset_proxy_enabled? + !context[:disable_asset_proxy] + end + + def asset_proxy_url(url) + "#{context[:asset_proxy]}/#{asset_url_hash(url)}/#{hexencode(url)}" + end + + def asset_url_hash(url) + OpenSSL::HMAC.hexdigest('sha1', context[:asset_proxy_secret_key], url) + end + + def hexencode(str) + str.unpack1('H*') + end end end end diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index cef72d24c43..bf23c74c0f0 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe AvatarsHelper do +RSpec.describe AvatarsHelper, feature_category: :source_code_management do include UploadHelpers let_it_be(:user) { create(:user) } @@ -88,7 +88,7 @@ RSpec.describe AvatarsHelper do describe '#avatar_icon_for' do let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') } let(:email) { 'foo@example.com' } - let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) } + let!(:another_user) { create(:user, :public_email, avatar: File.open(uploaded_image_temp_path), email: email) } it 'prefers the user to retrieve the avatar_url' do expect(helper.avatar_icon_for(user, email).to_s) @@ -102,7 +102,7 @@ RSpec.describe AvatarsHelper do end describe '#avatar_icon_for_email', :clean_gitlab_redis_cache do - let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) } + let(:user) { create(:user, :public_email, avatar: File.open(uploaded_image_temp_path)) } subject { helper.avatar_icon_for_email(user.email).to_s } @@ -114,6 +114,14 @@ RSpec.describe AvatarsHelper do end end + context 'when a private email is used' do + it 'calls gravatar_icon' do + expect(helper).to receive(:gravatar_icon).with(user.commit_email, 20, 2) + + helper.avatar_icon_for_email(user.commit_email, 20, 2) + end + end + context 'when no user exists for the email' do it 'calls gravatar_icon' do expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2) @@ -136,7 +144,7 @@ RSpec.describe AvatarsHelper do it_behaves_like "returns avatar for email" it "caches the request" do - expect(User).to receive(:find_by_any_email).once.and_call_original + expect(User).to receive(:with_public_email).once.and_call_original expect(helper.avatar_icon_for_email(user.email).to_s).to eq(user.avatar.url) expect(helper.avatar_icon_for_email(user.email).to_s).to eq(user.avatar.url) diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb index 004c70c28f1..dc6ac52a8c2 100644 --- a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb +++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb @@ -80,6 +80,15 @@ RSpec.describe Banzai::Filter::AssetProxyFilter, feature_category: :team_plannin expect(doc.at_css('img')['data-canonical-src']).to eq src end + it 'replaces invalid URLs' do + src = '///example.com/test.png' + new_src = 'https://assets.example.com/3368d2c7b9bed775bdd1e811f36a4b80a0dcd8ab/2f2f2f6578616d706c652e636f6d2f746573742e706e67' + doc = filter(image(src), @context) + + expect(doc.at_css('img')['src']).to eq new_src + expect(doc.at_css('img')['data-canonical-src']).to eq src + end + it 'skips internal images' do src = "#{Gitlab.config.gitlab.url}/test.png" doc = filter(image(src), @context) diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb index 3ebe0798972..896f3beb7c2 100644 --- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb @@ -218,7 +218,7 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter, feature_category: :source_c # any path-only link will automatically be prefixed # with the path of its repository. # See: "build_relative_path" in "lib/banzai/filter/relative_link_filter.rb" - let(:user_with_avatar) { create(:user, :with_avatar, username: 'foobar') } + let(:user_with_avatar) { create(:user, :public_email, :with_avatar, username: 'foobar') } it 'returns a full path for avatar urls' do _, message_html = build_commit_message( -- cgit v1.2.1 From 5e98d2784081393aea84b6591116d905da6eb567 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 1 May 2023 12:17:40 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-10-stable-ee --- .../import_entities/components/group_dropdown.vue | 4 +- .../import_groups/components/import_table.vue | 4 +- ...es_where_user_can_import_projects.query.graphql | 18 ++++ .../javascripts/projects/new/components/app.vue | 24 ++++- app/assets/javascripts/projects/new/index.js | 2 + app/controllers/import/base_controller.rb | 2 +- app/controllers/import/bitbucket_controller.rb | 4 +- app/controllers/import/gitea_controller.rb | 2 +- app/controllers/import/github_controller.rb | 2 +- .../import/gitlab_projects_controller.rb | 2 +- app/controllers/import/manifest_controller.rb | 4 +- app/controllers/projects/imports_controller.rb | 2 +- .../groups/accepting_project_imports_finder.rb | 31 ++++++ app/finders/groups/user_groups_finder.rb | 6 ++ app/graphql/types/permission_types/group_enum.rb | 3 + app/policies/group_policy.rb | 11 +- app/policies/namespaces/user_namespace_policy.rb | 5 +- app/services/import/base_service.rb | 2 +- app/services/import/bitbucket_server_service.rb | 2 +- app/services/import/fogbugz_service.rb | 4 +- app/services/import/github_service.rb | 2 +- app/services/projects/create_service.rb | 8 ++ app/views/projects/new.html.haml | 3 +- doc/api/graphql/reference/index.md | 1 + locale/gitlab.pot | 8 +- .../import/bitbucket_controller_spec.rb | 13 ++- .../import/bitbucket_server_controller_spec.rb | 2 +- spec/controllers/import/fogbugz_controller_spec.rb | 2 +- spec/controllers/import/gitea_controller_spec.rb | 2 +- spec/controllers/import/gitlab_controller_spec.rb | 2 +- .../controllers/import/manifest_controller_spec.rb | 4 +- .../projects/imports_controller_spec.rb | 14 +-- .../accepting_project_imports_finder_spec.rb | 105 ++++++++++++++++++ spec/finders/groups/user_groups_finder_spec.rb | 20 +++- .../components/group_dropdown_spec.js | 4 +- .../import_groups/components/import_table_spec.js | 4 +- .../components/import_target_cell_spec.js | 4 +- spec/frontend/projects/new/components/app_spec.js | 16 +++ spec/policies/group_policy_spec.rb | 118 +++++++++++++++++++++ .../namespaces/user_namespace_policy_spec.rb | 28 ++++- .../import/gitlab_projects_controller_spec.rb | 12 +++ .../import/bitbucket_server_service_spec.rb | 2 +- spec/services/import/fogbugz_service_spec.rb | 2 +- spec/services/import/github_service_spec.rb | 2 +- spec/services/projects/create_service_spec.rb | 17 +++ .../githubish_import_controller_shared_examples.rb | 13 +++ .../import_controller_status_shared_examples.rb | 22 ++++ 47 files changed, 504 insertions(+), 60 deletions(-) create mode 100644 app/assets/javascripts/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql create mode 100644 app/finders/groups/accepting_project_imports_finder.rb create mode 100644 spec/finders/groups/accepting_project_imports_finder_spec.rb diff --git a/app/assets/javascripts/import_entities/components/group_dropdown.vue b/app/assets/javascripts/import_entities/components/group_dropdown.vue index 5b9e80f9d68..1c31c04a416 100644 --- a/app/assets/javascripts/import_entities/components/group_dropdown.vue +++ b/app/assets/javascripts/import_entities/components/group_dropdown.vue @@ -4,7 +4,7 @@ import { debounce } from 'lodash'; import { s__ } from '~/locale'; import { createAlert } from '~/alert'; -import searchNamespacesWhereUserCanCreateProjectsQuery from '~/projects/new/queries/search_namespaces_where_user_can_create_projects.query.graphql'; +import searchNamespacesWhereUserCanImportProjectsQuery from '~/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql'; import { DEBOUNCE_DELAY } from '~/vue_shared/components/filtered_search_bar/constants'; import { MINIMUM_SEARCH_LENGTH } from '~/graphql_shared/constants'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; @@ -28,7 +28,7 @@ export default { }, apollo: { namespaces: { - query: searchNamespacesWhereUserCanCreateProjectsQuery, + query: searchNamespacesWhereUserCanImportProjectsQuery, variables() { return { search: this.searchTerm, diff --git a/app/assets/javascripts/import_entities/import_groups/components/import_table.vue b/app/assets/javascripts/import_entities/import_groups/components/import_table.vue index 2e6e7cddf8f..246d27d3b94 100644 --- a/app/assets/javascripts/import_entities/import_groups/components/import_table.vue +++ b/app/assets/javascripts/import_entities/import_groups/components/import_table.vue @@ -24,7 +24,7 @@ import { getGroupPathAvailability } from '~/rest_api'; import axios from '~/lib/utils/axios_utils'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import { helpPagePath } from '~/helpers/help_page_helper'; -import searchNamespacesWhereUserCanCreateProjectsQuery from '~/projects/new/queries/search_namespaces_where_user_can_create_projects.query.graphql'; +import searchNamespacesWhereUserCanImportProjectsQuery from '~/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { STATUSES } from '../../constants'; @@ -118,7 +118,7 @@ export default { }, }, availableNamespaces: { - query: searchNamespacesWhereUserCanCreateProjectsQuery, + query: searchNamespacesWhereUserCanImportProjectsQuery, update(data) { return data.currentUser.groups.nodes; }, diff --git a/app/assets/javascripts/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql b/app/assets/javascripts/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql new file mode 100644 index 00000000000..8c41f7116b3 --- /dev/null +++ b/app/assets/javascripts/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql @@ -0,0 +1,18 @@ +query searchNamespacesWhereUserCanImportProjects($search: String) { + currentUser { + id + groups(permissionScope: IMPORT_PROJECTS, search: $search) { + nodes { + id + fullPath + name + visibility + webUrl + } + } + namespace { + id + fullPath + } + } +} diff --git a/app/assets/javascripts/projects/new/components/app.vue b/app/assets/javascripts/projects/new/components/app.vue index 1599661505f..ec0742f7792 100644 --- a/app/assets/javascripts/projects/new/components/app.vue +++ b/app/assets/javascripts/projects/new/components/app.vue @@ -9,6 +9,7 @@ import NewNamespacePage from '~/vue_shared/new_namespace/new_namespace_page.vue' import NewProjectPushTipPopover from './new_project_push_tip_popover.vue'; const CI_CD_PANEL = 'cicd_for_external_repo'; +const IMPORT_PROJECT_PANEL = 'import_project'; const PANELS = [ { key: 'blank', @@ -32,7 +33,7 @@ const PANELS = [ }, { key: 'import', - name: 'import_project', + name: IMPORT_PROJECT_PANEL, selector: '#import-project-pane', title: s__('ProjectsNew|Import project'), description: s__( @@ -88,6 +89,11 @@ export default { required: false, default: '', }, + canImportProjects: { + type: Boolean, + required: false, + default: true, + }, }, computed: { @@ -100,7 +106,21 @@ export default { ]; }, availablePanels() { - return this.isCiCdAvailable ? PANELS : PANELS.filter((p) => p.name !== CI_CD_PANEL); + if (this.isCiCdAvailable && this.canImportProjects) { + return PANELS; + } + + return PANELS.filter((panel) => { + if (!this.canImportProjects && panel.name === IMPORT_PROJECT_PANEL) { + return false; + } + + if (!this.isCiCdAvailable && panel.name === CI_CD_PANEL) { + return false; + } + + return true; + }); }, }, diff --git a/app/assets/javascripts/projects/new/index.js b/app/assets/javascripts/projects/new/index.js index 7330874eefe..5ed2a90b089 100644 --- a/app/assets/javascripts/projects/new/index.js +++ b/app/assets/javascripts/projects/new/index.js @@ -18,6 +18,7 @@ export function initNewProjectCreation() { parentGroupUrl, parentGroupName, projectsUrl, + canImportProjects, } = el.dataset; const props = { @@ -27,6 +28,7 @@ export function initNewProjectCreation() { parentGroupUrl, parentGroupName, projectsUrl, + canImportProjects: parseBoolean(canImportProjects), }; const provide = { diff --git a/app/controllers/import/base_controller.rb b/app/controllers/import/base_controller.rb index 7ef07032913..bcb6aed9e38 100644 --- a/app/controllers/import/base_controller.rb +++ b/app/controllers/import/base_controller.rb @@ -18,7 +18,7 @@ class Import::BaseController < ApplicationController if params[:namespace_id]&.present? @namespace = Namespace.find_by_id(params[:namespace_id]) - render_404 unless current_user.can?(:create_projects, @namespace) + render_404 unless current_user.can?(:import_projects, @namespace) end end end diff --git a/app/controllers/import/bitbucket_controller.rb b/app/controllers/import/bitbucket_controller.rb index 8a0f4a36781..c933b05e0c4 100644 --- a/app/controllers/import/bitbucket_controller.rb +++ b/app/controllers/import/bitbucket_controller.rb @@ -57,7 +57,7 @@ class Import::BitbucketController < Import::BaseController extra: { user_role: user_role(current_user, target_namespace), import_type: 'bitbucket' } ) - if current_user.can?(:create_projects, target_namespace) + if current_user.can?(:import_projects, target_namespace) # The token in a session can be expired, we need to get most recent one because # Bitbucket::Connection class refreshes it. session[:bitbucket_token] = bitbucket_client.connection.token @@ -70,7 +70,7 @@ class Import::BitbucketController < Import::BaseController render json: { errors: project_save_error(project) }, status: :unprocessable_entity end else - render json: { errors: _('This namespace has already been taken! Please choose another one.') }, status: :unprocessable_entity + render json: { errors: _('You are not allowed to import projects in this namespace.') }, status: :unprocessable_entity end end diff --git a/app/controllers/import/gitea_controller.rb b/app/controllers/import/gitea_controller.rb index 047c273969c..2778b97419a 100644 --- a/app/controllers/import/gitea_controller.rb +++ b/app/controllers/import/gitea_controller.rb @@ -32,7 +32,7 @@ class Import::GiteaController < Import::GithubController if params[:namespace_id].present? @namespace = Namespace.find_by_id(params[:namespace_id]) - render_404 unless current_user.can?(:create_projects, @namespace) + render_404 unless current_user.can?(:import_projects, @namespace) end end end diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index f0a80593926..ba355686bc3 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -61,7 +61,7 @@ class Import::GithubController < Import::BaseController if params[:namespace_id].present? @namespace = Namespace.find_by_id(params[:namespace_id]) - render_404 unless current_user.can?(:create_projects, @namespace) + render_404 unless current_user.can?(:import_projects, @namespace) end end end diff --git a/app/controllers/import/gitlab_projects_controller.rb b/app/controllers/import/gitlab_projects_controller.rb index 9b8c480e529..d1b182a57d8 100644 --- a/app/controllers/import/gitlab_projects_controller.rb +++ b/app/controllers/import/gitlab_projects_controller.rb @@ -8,7 +8,7 @@ class Import::GitlabProjectsController < Import::BaseController def new @namespace = Namespace.find(project_params[:namespace_id]) - return render_404 unless current_user.can?(:create_projects, @namespace) + return render_404 unless current_user.can?(:import_projects, @namespace) @path = project_params[:path] end diff --git a/app/controllers/import/manifest_controller.rb b/app/controllers/import/manifest_controller.rb index 461ba982969..03884717e54 100644 --- a/app/controllers/import/manifest_controller.rb +++ b/app/controllers/import/manifest_controller.rb @@ -20,8 +20,8 @@ class Import::ManifestController < Import::BaseController def upload group = Group.find(params[:group_id]) - unless can?(current_user, :create_projects, group) - @errors = ["You don't have enough permissions to create projects in the selected group"] + unless can?(current_user, :import_projects, group) + @errors = ["You don't have enough permissions to import projects in the selected group"] render :new && return end diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb index 41daeddcf7f..208fbc40556 100644 --- a/app/controllers/projects/imports_controller.rb +++ b/app/controllers/projects/imports_controller.rb @@ -56,7 +56,7 @@ class Projects::ImportsController < Projects::ApplicationController end def require_namespace_project_creation_permission - render_404 unless can?(current_user, :admin_project, @project) || can?(current_user, :create_projects, @project.namespace) + render_404 unless can?(current_user, :admin_project, @project) || can?(current_user, :import_projects, @project.namespace) end def redirect_if_progress diff --git a/app/finders/groups/accepting_project_imports_finder.rb b/app/finders/groups/accepting_project_imports_finder.rb new file mode 100644 index 00000000000..55d72edf7bb --- /dev/null +++ b/app/finders/groups/accepting_project_imports_finder.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Groups + class AcceptingProjectImportsFinder + def initialize(current_user) + @current_user = current_user + end + + def execute + ::Group.from_union( + [ + current_user.manageable_groups, + managable_groups_originating_from_group_shares + ] + ) + end + + private + + attr_reader :current_user + + def managable_groups_originating_from_group_shares + GroupGroupLink + .with_owner_or_maintainer_access + .groups_accessible_via( + current_user.owned_or_maintainers_groups + .select(:id) + ) + end + end +end diff --git a/app/finders/groups/user_groups_finder.rb b/app/finders/groups/user_groups_finder.rb index b58c1323b1f..61f3e18bb99 100644 --- a/app/finders/groups/user_groups_finder.rb +++ b/app/finders/groups/user_groups_finder.rb @@ -39,6 +39,8 @@ module Groups target_user.manageable_groups(include_groups_with_developer_maintainer_access: true) elsif permission_scope_transfer_projects? Groups::AcceptingProjectTransfersFinder.new(target_user).execute # rubocop: disable CodeReuse/Finder + elsif permission_scope_import_projects? + Groups::AcceptingProjectImportsFinder.new(target_user).execute # rubocop: disable CodeReuse/Finder else target_user.groups end @@ -51,5 +53,9 @@ module Groups def permission_scope_transfer_projects? params[:permission_scope] == :transfer_projects end + + def permission_scope_import_projects? + params[:permission_scope] == :import_projects + end end end diff --git a/app/graphql/types/permission_types/group_enum.rb b/app/graphql/types/permission_types/group_enum.rb index f636d43790f..6d51d94a70d 100644 --- a/app/graphql/types/permission_types/group_enum.rb +++ b/app/graphql/types/permission_types/group_enum.rb @@ -10,6 +10,9 @@ module Types value 'TRANSFER_PROJECTS', value: :transfer_projects, description: 'Groups where the user can transfer projects to.' + value 'IMPORT_PROJECTS', + value: :import_projects, + description: 'Groups where the user can import projects to.' end end end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index ee1140b8405..94a1c01fa8c 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -190,6 +190,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :destroy_package enable :admin_package enable :create_projects + enable :import_projects enable :admin_pipeline enable :admin_build enable :add_cluster @@ -260,14 +261,20 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy end.enable :change_share_with_group_lock rule { developer & developer_maintainer_access }.enable :create_projects - rule { create_projects_disabled }.prevent :create_projects + rule { create_projects_disabled }.policy do + prevent :create_projects + prevent :import_projects + end rule { owner | admin }.policy do enable :owner_access enable :read_statistics end - rule { maintainer & can?(:create_projects) }.enable :transfer_projects + rule { maintainer & can?(:create_projects) }.policy do + enable :transfer_projects + enable :import_projects + end rule { read_package_registry_deploy_token }.policy do enable :read_package diff --git a/app/policies/namespaces/user_namespace_policy.rb b/app/policies/namespaces/user_namespace_policy.rb index 1deeae8241f..bfed61e72d3 100644 --- a/app/policies/namespaces/user_namespace_policy.rb +++ b/app/policies/namespaces/user_namespace_policy.rb @@ -11,6 +11,7 @@ module Namespaces rule { owner | admin }.policy do enable :owner_access enable :create_projects + enable :import_projects enable :admin_namespace enable :read_namespace enable :read_statistics @@ -20,9 +21,9 @@ module Namespaces enable :edit_billing end - rule { ~can_create_personal_project }.prevent :create_projects + rule { ~can_create_personal_project }.prevent :create_projects, :import_projects - rule { bot_user_namespace }.prevent :create_projects + rule { bot_user_namespace }.prevent :create_projects, :import_projects rule { (owner | admin) & can?(:create_projects) }.enable :transfer_projects end diff --git a/app/services/import/base_service.rb b/app/services/import/base_service.rb index 6b5adcbc39e..64cf3cfa04a 100644 --- a/app/services/import/base_service.rb +++ b/app/services/import/base_service.rb @@ -9,7 +9,7 @@ module Import end def authorized? - can?(current_user, :create_projects, target_namespace) + can?(current_user, :import_projects, target_namespace) end private diff --git a/app/services/import/bitbucket_server_service.rb b/app/services/import/bitbucket_server_service.rb index f7f17f1e53e..5d496dc7cc3 100644 --- a/app/services/import/bitbucket_server_service.rb +++ b/app/services/import/bitbucket_server_service.rb @@ -10,7 +10,7 @@ module Import end unless authorized? - return log_and_return_error("You don't have permissions to create this project", :unauthorized) + return log_and_return_error("You don't have permissions to import this project", :unauthorized) end unless repo diff --git a/app/services/import/fogbugz_service.rb b/app/services/import/fogbugz_service.rb index d1003823456..9a8def43312 100644 --- a/app/services/import/fogbugz_service.rb +++ b/app/services/import/fogbugz_service.rb @@ -13,8 +13,8 @@ module Import unless authorized? return log_and_return_error( - "You don't have permissions to create this project", - _("You don't have permissions to create this project"), + "You don't have permissions to import this project", + _("You don't have permissions to import this project"), :unauthorized ) end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index b30c344723d..7e7f7ea9810 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -103,7 +103,7 @@ module Import elsif target_namespace.nil? error(_('Namespace or group to import repository into does not exist.'), :unprocessable_entity) elsif !authorized? - error(_('This namespace has already been taken. Choose a different one.'), :unprocessable_entity) + error(_('You are not allowed to import projects in this namespace.'), :unprocessable_entity) elsif oversized? error(oversize_error_message, :unprocessable_entity) end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 94cc4700a49..b535fce3fc5 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -58,6 +58,7 @@ module Projects return @project if @project.errors.any? validate_create_permissions + validate_import_permissions return @project if @project.errors.any? @relations_block&.call(@project) @@ -98,6 +99,13 @@ module Projects @project.errors.add(:namespace, "is not valid") end + def validate_import_permissions + return unless @project.import? + return if current_user.can?(:import_projects, parent_namespace) + + @project.errors.add(:user, 'is not allowed to import projects') + end + def after_create_actions log_info("#{current_user.name} created a new project \"#{@project.full_name}\"") diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index f4a5862b2c0..323353f6cc0 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -16,7 +16,8 @@ working_with_projects_help_path: help_page_path("user/project/working_with_projects"), parent_group_url: @project.parent && group_url(@project.parent), parent_group_name: @project.parent&.name, - projects_url: dashboard_projects_url } } + projects_url: dashboard_projects_url, + can_import_projects: params[:namespace_id].presence ? current_user.can?(:import_projects, @namespace).to_s : 'true' } } .row{ 'v-cloak': true } #blank-project-pane.tab-pane.active diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index bcd7c9dde44..f285c872cde 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -23353,6 +23353,7 @@ User permission on groups. | Value | Description | | ----- | ----------- | | `CREATE_PROJECTS` | Groups where the user can create projects. | +| `IMPORT_PROJECTS` | Groups where the user can import projects to. | | `TRANSFER_PROJECTS` | Groups where the user can transfer projects to. | ### `GroupReleaseSort` diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6da296d5381..f9dfad444e0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -44806,9 +44806,6 @@ msgstr "" msgid "This namespace has already been taken! Please choose another one." msgstr "" -msgid "This namespace has already been taken. Choose a different one." -msgstr "" - msgid "This only applies to repository indexing operations." msgstr "" @@ -49706,6 +49703,9 @@ msgstr "" msgid "You are not allowed to download code from this project." msgstr "" +msgid "You are not allowed to import projects in this namespace." +msgstr "" + msgid "You are not allowed to log in using password" msgstr "" @@ -50018,7 +50018,7 @@ msgstr "" msgid "You don't have permission to view this epic" msgstr "" -msgid "You don't have permissions to create this project" +msgid "You don't have permissions to import this project" msgstr "" msgid "You don't have sufficient permission to perform this action." diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index 055c98ebdbc..906cc5cb336 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::BitbucketController do +RSpec.describe Import::BitbucketController, feature_category: :importers do include ImportSpecHelper let(:user) { create(:user) } @@ -445,5 +445,16 @@ RSpec.describe Import::BitbucketController do ) end end + + context 'when user can not import projects' do + let!(:other_namespace) { create(:group, name: 'other_namespace').tap { |other_namespace| other_namespace.add_developer(user) } } + + it 'returns 422 response' do + post :create, params: { target_namespace: other_namespace.name }, format: :json + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.parsed_body['errors']).to eq('You are not allowed to import projects in this namespace.') + end + end end end diff --git a/spec/controllers/import/bitbucket_server_controller_spec.rb b/spec/controllers/import/bitbucket_server_controller_spec.rb index ac56d3af54f..b2a56423253 100644 --- a/spec/controllers/import/bitbucket_server_controller_spec.rb +++ b/spec/controllers/import/bitbucket_server_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::BitbucketServerController do +RSpec.describe Import::BitbucketServerController, feature_category: :importers do let(:user) { create(:user) } let(:project_key) { 'test-project' } let(:repo_slug) { 'some-repo' } diff --git a/spec/controllers/import/fogbugz_controller_spec.rb b/spec/controllers/import/fogbugz_controller_spec.rb index e2d59fc213a..40a5c59fa2d 100644 --- a/spec/controllers/import/fogbugz_controller_spec.rb +++ b/spec/controllers/import/fogbugz_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::FogbugzController do +RSpec.describe Import::FogbugzController, feature_category: :importers do include ImportSpecHelper let(:user) { create(:user) } diff --git a/spec/controllers/import/gitea_controller_spec.rb b/spec/controllers/import/gitea_controller_spec.rb index 568712d29cb..7466ffb2393 100644 --- a/spec/controllers/import/gitea_controller_spec.rb +++ b/spec/controllers/import/gitea_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::GiteaController do +RSpec.describe Import::GiteaController, feature_category: :importers do include ImportSpecHelper let(:provider) { :gitea } diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index 7b3978297fb..2c09f8c010e 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::GitlabController do +RSpec.describe Import::GitlabController, feature_category: :importers do include ImportSpecHelper let(:user) { create(:user) } diff --git a/spec/controllers/import/manifest_controller_spec.rb b/spec/controllers/import/manifest_controller_spec.rb index 6f805b44e89..23d5d37ed88 100644 --- a/spec/controllers/import/manifest_controller_spec.rb +++ b/spec/controllers/import/manifest_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state do +RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state, feature_category: :importers do include ImportSpecHelper let_it_be(:user) { create(:user) } @@ -45,7 +45,7 @@ RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state do end end - context 'when the user cannot create projects in the group' do + context 'when the user cannot import projects in the group' do it 'displays an error' do sign_in(create(:user)) diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index 65a80b9e8ec..05232afb81a 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::ImportsController do +RSpec.describe Projects::ImportsController, feature_category: :importers do let(:user) { create(:user) } let(:project) { create(:project) } @@ -149,17 +149,7 @@ RSpec.describe Projects::ImportsController do import_state.update!(status: :started) end - context 'when group allows developers to create projects' do - let(:group) { create(:group, project_creation_level: Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) } - - it 'renders template' do - get :show, params: { namespace_id: project.namespace.to_param, project_id: project } - - expect(response).to render_template :show - end - end - - context 'when group prohibits developers to create projects' do + context 'when group prohibits developers to import projects' do let(:group) { create(:group, project_creation_level: Gitlab::Access::MAINTAINER_PROJECT_ACCESS) } it 'returns 404 response' do diff --git a/spec/finders/groups/accepting_project_imports_finder_spec.rb b/spec/finders/groups/accepting_project_imports_finder_spec.rb new file mode 100644 index 00000000000..4e06c2cbc67 --- /dev/null +++ b/spec/finders/groups/accepting_project_imports_finder_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::AcceptingProjectImportsFinder, feature_category: :importers do + let_it_be(:user) { create(:user) } + let_it_be(:group_where_direct_owner) { create(:group) } + let_it_be(:subgroup_of_group_where_direct_owner) { create(:group, parent: group_where_direct_owner) } + let_it_be(:group_where_direct_maintainer) { create(:group) } + let_it_be(:group_where_direct_maintainer_but_cant_create_projects) do + create(:group, project_creation_level: Gitlab::Access::NO_ONE_PROJECT_ACCESS) + end + + let_it_be(:group_where_direct_developer_but_developers_cannot_create_projects) { create(:group) } + let_it_be(:group_where_direct_developer) do + create(:group, project_creation_level: Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) + end + + let_it_be(:shared_with_group_where_direct_owner_as_owner) { create(:group) } + + let_it_be(:shared_with_group_where_direct_owner_as_developer) do + create(:group, project_creation_level: Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) + end + + let_it_be(:shared_with_group_where_direct_owner_as_developer_but_developers_cannot_create_projects) do + create(:group) + end + + let_it_be(:shared_with_group_where_direct_developer_as_maintainer) do + create(:group, project_creation_level: Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) + end + + let_it_be(:shared_with_group_where_direct_owner_as_guest) { create(:group) } + let_it_be(:shared_with_group_where_direct_owner_as_maintainer) { create(:group) } + let_it_be(:shared_with_group_where_direct_developer_as_owner) do + create(:group, project_creation_level: Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) + end + + let_it_be(:subgroup_of_shared_with_group_where_direct_owner_as_maintainer) do + create(:group, parent: shared_with_group_where_direct_owner_as_maintainer) + end + + before do + group_where_direct_owner.add_owner(user) + group_where_direct_maintainer.add_maintainer(user) + group_where_direct_developer_but_developers_cannot_create_projects.add_developer(user) + group_where_direct_developer.add_developer(user) + + create(:group_group_link, :owner, + shared_with_group: group_where_direct_owner, + shared_group: shared_with_group_where_direct_owner_as_owner + ) + + create(:group_group_link, :developer, + shared_with_group: group_where_direct_owner, + shared_group: shared_with_group_where_direct_owner_as_developer_but_developers_cannot_create_projects + ) + + create(:group_group_link, :maintainer, + shared_with_group: group_where_direct_developer, + shared_group: shared_with_group_where_direct_developer_as_maintainer + ) + + create(:group_group_link, :developer, + shared_with_group: group_where_direct_owner, + shared_group: shared_with_group_where_direct_owner_as_developer + ) + + create(:group_group_link, :guest, + shared_with_group: group_where_direct_owner, + shared_group: shared_with_group_where_direct_owner_as_guest + ) + + create(:group_group_link, :maintainer, + shared_with_group: group_where_direct_owner, + shared_group: shared_with_group_where_direct_owner_as_maintainer + ) + + create(:group_group_link, :owner, + shared_with_group: group_where_direct_developer_but_developers_cannot_create_projects, + shared_group: shared_with_group_where_direct_developer_as_owner + ) + end + + describe '#execute' do + subject(:result) { described_class.new(user).execute } + + it 'only returns groups where the user has access to import projects' do + expect(result).to match_array([ + group_where_direct_owner, + subgroup_of_group_where_direct_owner, + group_where_direct_maintainer, + # groups arising from group shares + shared_with_group_where_direct_owner_as_owner, + shared_with_group_where_direct_owner_as_maintainer, + subgroup_of_shared_with_group_where_direct_owner_as_maintainer + ]) + + expect(result).not_to include(group_where_direct_developer) + expect(result).not_to include(shared_with_group_where_direct_developer_as_owner) + expect(result).not_to include(shared_with_group_where_direct_developer_as_maintainer) + expect(result).not_to include(shared_with_group_where_direct_owner_as_developer) + end + end +end diff --git a/spec/finders/groups/user_groups_finder_spec.rb b/spec/finders/groups/user_groups_finder_spec.rb index 999079468e5..f6df396037c 100644 --- a/spec/finders/groups/user_groups_finder_spec.rb +++ b/spec/finders/groups/user_groups_finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Groups::UserGroupsFinder do +RSpec.describe Groups::UserGroupsFinder, feature_category: :subgroups do describe '#execute' do let_it_be(:user) { create(:user) } let_it_be(:root_group) { create(:group, name: 'Root group', path: 'root-group') } @@ -98,6 +98,24 @@ RSpec.describe Groups::UserGroupsFinder do end end + context 'when permission is :import_projects' do + let(:arguments) { { permission_scope: :import_projects } } + + specify do + is_expected.to contain_exactly( + public_maintainer_group, + public_owner_group, + private_maintainer_group + ) + end + + it_behaves_like 'user group finder searching by name or path' do + let(:keyword_search_expected_groups) do + [public_maintainer_group] + end + end + end + context 'when permission is :transfer_projects' do let(:arguments) { { permission_scope: :transfer_projects } } diff --git a/spec/frontend/import_entities/components/group_dropdown_spec.js b/spec/frontend/import_entities/components/group_dropdown_spec.js index b44bc33de6f..14f39a35387 100644 --- a/spec/frontend/import_entities/components/group_dropdown_spec.js +++ b/spec/frontend/import_entities/components/group_dropdown_spec.js @@ -6,7 +6,7 @@ import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import GroupDropdown from '~/import_entities/components/group_dropdown.vue'; import { DEBOUNCE_DELAY } from '~/vue_shared/components/filtered_search_bar/constants'; -import searchNamespacesWhereUserCanCreateProjectsQuery from '~/projects/new/queries/search_namespaces_where_user_can_create_projects.query.graphql'; +import searchNamespacesWhereUserCanImportProjectsQuery from '~/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql'; Vue.use(VueApollo); @@ -49,7 +49,7 @@ describe('Import entities group dropdown component', () => { const createComponent = (propsData) => { const apolloProvider = createMockApollo([ - [searchNamespacesWhereUserCanCreateProjectsQuery, () => SEARCH_NAMESPACES_MOCK], + [searchNamespacesWhereUserCanImportProjectsQuery, () => SEARCH_NAMESPACES_MOCK], ]); namespacesTracker = jest.fn(); diff --git a/spec/frontend/import_entities/import_groups/components/import_table_spec.js b/spec/frontend/import_entities/import_groups/components/import_table_spec.js index 205218fdabd..05e93f354c4 100644 --- a/spec/frontend/import_entities/import_groups/components/import_table_spec.js +++ b/spec/frontend/import_entities/import_groups/components/import_table_spec.js @@ -15,7 +15,7 @@ import ImportTable from '~/import_entities/import_groups/components/import_table import importGroupsMutation from '~/import_entities/import_groups/graphql/mutations/import_groups.mutation.graphql'; import PaginationBar from '~/vue_shared/components/pagination_bar/pagination_bar.vue'; import PaginationLinks from '~/vue_shared/components/pagination_links.vue'; -import searchNamespacesWhereUserCanCreateProjectsQuery from '~/projects/new/queries/search_namespaces_where_user_can_create_projects.query.graphql'; +import searchNamespacesWhereUserCanImportProjectsQuery from '~/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql'; import { AVAILABLE_NAMESPACES, @@ -74,7 +74,7 @@ describe('import table', () => { apolloProvider = createMockApollo( [ [ - searchNamespacesWhereUserCanCreateProjectsQuery, + searchNamespacesWhereUserCanImportProjectsQuery, () => Promise.resolve(availableNamespacesFixture), ], ], diff --git a/spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js b/spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js index a524d9ebdb0..a957e85723f 100644 --- a/spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js +++ b/spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js @@ -8,7 +8,7 @@ import ImportGroupDropdown from '~/import_entities/components/group_dropdown.vue import { STATUSES } from '~/import_entities/constants'; import ImportTargetCell from '~/import_entities/import_groups/components/import_target_cell.vue'; import { DEBOUNCE_DELAY } from '~/vue_shared/components/filtered_search_bar/constants'; -import searchNamespacesWhereUserCanCreateProjectsQuery from '~/projects/new/queries/search_namespaces_where_user_can_create_projects.query.graphql'; +import searchNamespacesWhereUserCanImportProjectsQuery from '~/import_entities/import_projects/graphql/queries/search_namespaces_where_user_can_import_projects.query.graphql'; import { generateFakeEntry, @@ -42,7 +42,7 @@ describe('import target cell', () => { const createComponent = (props) => { apolloProvider = createMockApollo([ [ - searchNamespacesWhereUserCanCreateProjectsQuery, + searchNamespacesWhereUserCanImportProjectsQuery, () => Promise.resolve(availableNamespacesFixture), ], ]); diff --git a/spec/frontend/projects/new/components/app_spec.js b/spec/frontend/projects/new/components/app_spec.js index 5b2dc25077e..079bd41cd37 100644 --- a/spec/frontend/projects/new/components/app_spec.js +++ b/spec/frontend/projects/new/components/app_spec.js @@ -41,6 +41,22 @@ describe('Experimental new project creation app', () => { ).toBe(isCiCdAvailable); }); + it.each` + canImportProjects | outcome + ${false} | ${'do not show Import panel'} + ${true} | ${'show Import panel'} + `('$outcome when canImportProjects is $canImportProjects', ({ canImportProjects }) => { + createComponent({ + canImportProjects, + }); + + expect( + findNewNamespacePage() + .props() + .panels.some((p) => p.name === 'import_project'), + ).toBe(canImportProjects); + }); + it('creates correct breadcrumbs for top-level projects', () => { createComponent(); diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 003ca2512dc..ad14c5c3f43 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -670,6 +670,124 @@ RSpec.describe GroupPolicy, feature_category: :system_access do end end + context 'import_projects' do + before do + group.update!(project_creation_level: project_creation_level) + end + + context 'when group has no project creation level set' do + let(:project_creation_level) { nil } + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + + context 'when group has project creation level set to no one' do + let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_disallowed(:import_projects) } + end + end + + context 'when group has project creation level set to maintainer only' do + let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + + context 'when group has project creation level set to developers + maintainer' do + let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS } + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + end + context 'create_subgroup' do context 'when group has subgroup creation level set to owner' do before do diff --git a/spec/policies/namespaces/user_namespace_policy_spec.rb b/spec/policies/namespaces/user_namespace_policy_spec.rb index bb821490e30..3488f33f15c 100644 --- a/spec/policies/namespaces/user_namespace_policy_spec.rb +++ b/spec/policies/namespaces/user_namespace_policy_spec.rb @@ -2,13 +2,13 @@ require 'spec_helper' -RSpec.describe Namespaces::UserNamespacePolicy do +RSpec.describe Namespaces::UserNamespacePolicy, feature_category: :subgroups do let_it_be(:user) { create(:user) } let_it_be(:owner) { create(:user) } let_it_be(:admin) { create(:admin) } let_it_be(:namespace) { create(:user_namespace, owner: owner) } - let(:owner_permissions) { [:owner_access, :create_projects, :admin_namespace, :read_namespace, :read_statistics, :transfer_projects, :admin_package, :read_billing, :edit_billing] } + let(:owner_permissions) { [:owner_access, :create_projects, :admin_namespace, :read_namespace, :read_statistics, :transfer_projects, :admin_package, :read_billing, :edit_billing, :import_projects] } subject { described_class.new(current_user, namespace) } @@ -34,6 +34,7 @@ RSpec.describe Namespaces::UserNamespacePolicy do it { is_expected.to be_disallowed(:create_projects) } it { is_expected.to be_disallowed(:transfer_projects) } + it { is_expected.to be_disallowed(:import_projects) } end context 'bot user' do @@ -41,6 +42,7 @@ RSpec.describe Namespaces::UserNamespacePolicy do it { is_expected.to be_disallowed(:create_projects) } it { is_expected.to be_disallowed(:transfer_projects) } + it { is_expected.to be_disallowed(:import_projects) } end end @@ -103,4 +105,26 @@ RSpec.describe Namespaces::UserNamespacePolicy do it { is_expected.to be_disallowed(:create_projects) } end end + + describe 'import projects' do + context 'when user can import projects' do + let(:current_user) { owner } + + before do + allow(current_user).to receive(:can_import_project?).and_return(true) + end + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'when user cannot create projects' do + let(:current_user) { user } + + before do + allow(current_user).to receive(:can_import_project?).and_return(false) + end + + it { is_expected.to be_disallowed(:import_projects) } + end + end end diff --git a/spec/requests/import/gitlab_projects_controller_spec.rb b/spec/requests/import/gitlab_projects_controller_spec.rb index b2c2d306e53..fe3ea9e9c9e 100644 --- a/spec/requests/import/gitlab_projects_controller_spec.rb +++ b/spec/requests/import/gitlab_projects_controller_spec.rb @@ -90,4 +90,16 @@ RSpec.describe Import::GitlabProjectsController, feature_category: :importers do subject { post authorize_import_gitlab_project_path, headers: workhorse_headers } end end + + describe 'GET new' do + context 'when the user is not allowed to import projects' do + let!(:group) { create(:group).tap { |group| group.add_developer(user) } } + + it 'returns 404' do + get new_import_gitlab_project_path, params: { namespace_id: group.id } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end diff --git a/spec/services/import/bitbucket_server_service_spec.rb b/spec/services/import/bitbucket_server_service_spec.rb index aea6c45b3a8..ca554fb01c3 100644 --- a/spec/services/import/bitbucket_server_service_spec.rb +++ b/spec/services/import/bitbucket_server_service_spec.rb @@ -93,7 +93,7 @@ RSpec.describe Import::BitbucketServerService, feature_category: :importers do result = subject.execute(credentials) expect(result).to include( - message: "You don't have permissions to create this project", + message: "You don't have permissions to import this project", status: :error, http_status: :unauthorized ) diff --git a/spec/services/import/fogbugz_service_spec.rb b/spec/services/import/fogbugz_service_spec.rb index 6953213add7..ad02dc31da1 100644 --- a/spec/services/import/fogbugz_service_spec.rb +++ b/spec/services/import/fogbugz_service_spec.rb @@ -61,7 +61,7 @@ RSpec.describe Import::FogbugzService, feature_category: :importers do result = subject.execute(credentials) expect(result).to include( - message: "You don't have permissions to create this project", + message: "You don't have permissions to import this project", status: :error, http_status: :unauthorized ) diff --git a/spec/services/import/github_service_spec.rb b/spec/services/import/github_service_spec.rb index 5d762568a62..a8928fb5c09 100644 --- a/spec/services/import/github_service_spec.rb +++ b/spec/services/import/github_service_spec.rb @@ -291,7 +291,7 @@ RSpec.describe Import::GithubService, feature_category: :importers do { status: :error, http_status: :unprocessable_entity, - message: 'This namespace has already been taken. Choose a different one.' + message: 'You are not allowed to import projects in this namespace.' } end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index e435db4efa6..24e52e6ea67 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -254,6 +254,23 @@ RSpec.describe Projects::CreateService, '#execute', feature_category: :projects end it_behaves_like 'has sync-ed traversal_ids' + + context 'when project is an import' do + context 'when user is not allowed to import projects' do + let(:group) do + create(:group).tap do |group| + group.add_developer(user) + end + end + + it 'does not create the project' do + project = create_project(user, opts.merge!(namespace_id: group.id, import_type: 'gitlab_project')) + + expect(project).not_to be_persisted + expect(project.errors.messages[:user].first).to eq('is not allowed to import projects') + end + end + end end context 'group sharing', :sidekiq_inline do diff --git a/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb b/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb index de38d1ff9f8..af1843bae28 100644 --- a/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb @@ -138,6 +138,19 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do .not_to exceed_all_query_limit(control_count) end + context 'when user is not allowed to import projects' do + let(:user) { create(:user) } + let!(:group) { create(:group).tap { |group| group.add_developer(user) } } + + it 'returns 404' do + expect(stub_client(repos: [], orgs: [])).to receive(:repos) + + get :status, params: { namespace_id: group.id }, format: :html + + expect(response).to have_gitlab_http_status(:not_found) + end + end + context 'when filtering' do let(:repo_2) { repo_fake.new(login: 'emacs', full_name: 'asd/emacs', name: 'emacs', owner: { login: 'owner' }) } let(:project) { create(:project, import_type: provider, namespace: user.namespace, import_status: :finished, import_source: 'example/repo') } diff --git a/spec/support/shared_examples/controllers/import_controller_status_shared_examples.rb b/spec/support/shared_examples/controllers/import_controller_status_shared_examples.rb index 44baadaaade..e94f063399d 100644 --- a/spec/support/shared_examples/controllers/import_controller_status_shared_examples.rb +++ b/spec/support/shared_examples/controllers/import_controller_status_shared_examples.rb @@ -19,4 +19,26 @@ RSpec.shared_examples 'import controller status' do expect(json_response.dig("imported_projects", 0, "id")).to eq(project.id) expect(json_response.dig("provider_repos", 0, "id")).to eq(repo_id) end + + context 'when format is html' do + context 'when namespace_id is present' do + let!(:developer_group) { create(:group).tap { |g| g.add_developer(user) } } + + context 'when user cannot import projects' do + it 'returns 404' do + get :status, params: { namespace_id: developer_group.id }, format: :html + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user can import projects' do + it 'returns 200' do + get :status, params: { namespace_id: group.id }, format: :html + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end end -- cgit v1.2.1 From b3720499932d69dc76318db1960618b0ad6cf6db Mon Sep 17 00:00:00 2001 From: GitLab Release Tools Bot Date: Mon, 1 May 2023 15:51:46 +0000 Subject: Update VERSION files [ci skip] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 597ea7a6e31..3c46dc56587 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -15.10.4 \ No newline at end of file +15.10.5 \ No newline at end of file -- cgit v1.2.1 From 22f3fab9f647bb1ea6e19330b5ca0e877d7ff344 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 1 May 2023 15:54:58 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-10-stable-ee --- CHANGELOG.md | 14 ++++++++++++++ GITALY_SERVER_VERSION | 2 +- GITLAB_PAGES_VERSION | 2 +- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fd9c06984b..c82d7270e75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.10.5 (2023-05-01) + +### Security (9 changes) + +- [Set minimum role for importing projects to Maintainer](gitlab-org/security/gitlab@d4cff7e53961d819b30ae748a38e4c8e4d856b32) ([merge request](gitlab-org/security/gitlab!3215)) +- [Commit trailers now only match public user email addresses](gitlab-org/security/gitlab@4948acdb39ba6ae9a71ef133e38ec47327d14f97) ([merge request](gitlab-org/security/gitlab!3208)) +- [Handle invalid URLs in asset proxy](gitlab-org/security/gitlab@b22e923ab3d48d9389311192d92dd89e2bfc24aa) ([merge request](gitlab-org/security/gitlab!3212)) +- [Relay state to check for only allowing sub paths](gitlab-org/security/gitlab@24f84fafd65dfedf36e859d305dd46bf3e71c8dc) ([merge request](gitlab-org/security/gitlab!3220)) +- [Prohibit 40 character hex sets at beginning of path-based branch name](gitlab-org/security/gitlab@71d30b6537f6853fef45acba16ab26b6f32718f7) ([merge request](gitlab-org/security/gitlab!3194)) +- [Add specs for external users flag](gitlab-org/security/gitlab@dfdb540285e573bd55a8647db4de8370ba6b3286) ([merge request](gitlab-org/security/gitlab!3190)) +- [Update policy to prevent banned members from accessing public projects](gitlab-org/security/gitlab@bc211b8be25e56f35c80d2331447f251c7a7dd56) ([merge request](gitlab-org/security/gitlab!3186)) +- [Use dummy filename as filename when viewing raw xml files](gitlab-org/security/gitlab@6d871f56d7a343d705f8c849d24a94b3528c3a97) ([merge request](gitlab-org/security/gitlab!3192)) +- [Authorize access to vulnerabilitiesCountByDay resolver](gitlab-org/security/gitlab@888c187aab7c7062ea43b61a282c4dea8c6a47be) ([merge request](gitlab-org/security/gitlab!3180)) + ## 15.10.4 (2023-04-21) ### Fixed (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 597ea7a6e31..3c46dc56587 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -15.10.4 \ No newline at end of file +15.10.5 \ No newline at end of file diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 597ea7a6e31..3c46dc56587 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -15.10.4 \ No newline at end of file +15.10.5 \ No newline at end of file -- cgit v1.2.1