Merge remote-tracking branch 'dev/master'

CHANGELOG.md

@@ -2,6 +2,20 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 14.2.2 (2021-08-31)
+
+### Security (9 changes)
+
+- [Prevent non-admins from configuring Jira connect app](gitlab-org/security/gitlab@1bc56361c9daa90accea65836d5a424168a2c544) ([merge request](gitlab-org/security/gitlab!1697))
+- [Only create jira connect NS subscriptions for admins](gitlab-org/security/gitlab@c160da2cb32a5774fef149155cfd397981bf9173) ([merge request](gitlab-org/security/gitlab!1698))
+- [Update apollo_upload_server dependency](gitlab-org/security/gitlab@5ef659b8c9a5a7338830171c62943d3b8bb16410) ([merge request](gitlab-org/security/gitlab!1699))
+- [Ensure shared group members lose project access after group deletion](gitlab-org/security/gitlab@c94e934234a90f82e7fe291ed0f1d6a763b9a977) ([merge request](gitlab-org/security/gitlab!1683))
+- [Update Import/Export to use public email when mapping users](gitlab-org/security/gitlab@13fb902c55c2dfe7ec2bf35f58a9cb3d93905d9a) ([merge request](gitlab-org/security/gitlab!1669)) **GitLab Enterprise Edition**
+- [Require sign in for .keys endpoint on non-public instances](gitlab-org/security/gitlab@0979dd458e8fa0d4f5e184ef0b9ea042d79f6c14) ([merge request](gitlab-org/security/gitlab!1676))
+- [Inherit user external status while creating project bots](gitlab-org/security/gitlab@93062909ffc093cb8f718a3ea3f2976292a9b9af) ([merge request](gitlab-org/security/gitlab!1675))
+- [Escape issue reference and title for Jira issues](gitlab-org/security/gitlab@d25ef8599ec03ee80ef1bff7067b2269836400cf) ([merge request](gitlab-org/security/gitlab!1673)) **GitLab Enterprise Edition**
+- [Fix stored XSS vulnerability in Datadog settings form](gitlab-org/security/gitlab@23b98dac7864992898992a153950247ac6ccb933) ([merge request](gitlab-org/security/gitlab!1670))
+
 ## 14.2.1 (2021-08-23)
 
 ### Fixed (1 change)
@@ -563,6 +577,19 @@ entry.
 - [Add helpful text to URL group validation and limit text](gitlab-org/gitlab@59a5a6266cb0d5434596170ffa36e4e74b8d2c2c) ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65369)) **GitLab Enterprise Edition**
 - [Refactor external storage admin area configuration UI and docs](gitlab-org/gitlab@497ba4fc8f4ec1d234c9f5f1ec5c69712b8c7cb3) ([merge request](gitlab-org/gitlab!66219))
 
+## 14.1.4 (2021-08-31)
+
+### Security (8 changes)
+
+- [Update apollo_upload_server dependency](gitlab-org/security/gitlab@34e7e3b7590fd76d0f618091551651e8065edfd2) ([merge request](gitlab-org/security/gitlab!1700))
+- [Ensure shared group members lose project access after group deletion](gitlab-org/security/gitlab@4a7b8203776b719c06186c1b189a8cf21572fcd4) ([merge request](gitlab-org/security/gitlab!1684))
+- [Fix stored XSS vulnerability in Datadog settings form](gitlab-org/security/gitlab@0906814af604e7fcab54a96bccadcba11207387d) ([merge request](gitlab-org/security/gitlab!1671))
+- [Inherit user external status while creating project bots](gitlab-org/security/gitlab@d5a26c4145d917b5b49e207e03669d2b7e4ee617) ([merge request](gitlab-org/security/gitlab!1665))
+- [Escape issue reference and title for Jira issues](gitlab-org/security/gitlab@4153444b76421ddf3a7fd21f1fc0500700a4e263) ([merge request](gitlab-org/security/gitlab!1662)) **GitLab Enterprise Edition**
+- [Require sign in for .keys endpoint on non-public instances](gitlab-org/security/gitlab@b090b3f6dee6d21d93595c5e46e6c5c7fc30f1fb) ([merge request](gitlab-org/security/gitlab!1658))
+- [Only create jira connect NS subscriptions for admins](gitlab-org/security/gitlab@3f2040c0e2c90f3fcafdbf0f86bd2591bd458dff) ([merge request](gitlab-org/security/gitlab!1648))
+- [Prevent non-admins from configuring Jira connect app](gitlab-org/security/gitlab@fa864c0a2eaf450033f4c594cea07d9f24144cd6) ([merge request](gitlab-org/security/gitlab!1644))
+
 ## 14.1.3 (2021-08-17)
 
 ### Fixed (2 changes)
@@ -1181,6 +1208,20 @@ entry.
 - [Remove diffs gradual load feature flag](gitlab-org/gitlab@027d7c4327b5b6205a84281239027273517bf81b) ([merge request](gitlab-org/gitlab!55478))
 - [Remove partial index for Hashed Storage migration](gitlab-org/gitlab@3ed017a1023d7b0941a7606b69e6caee8d22f15c) ([merge request](gitlab-org/gitlab!62920))
 
+## 14.0.9 (2021-08-31)
+
+### Security (9 changes)
+
+- [Update apollo_upload_server dependency](gitlab-org/security/gitlab@ced741d93fa664f0c152f524949258bf969b7667) ([merge request](gitlab-org/security/gitlab!1701))
+- [Ensure shared group members lose project access after group deletion](gitlab-org/security/gitlab@3a41f4e29c01188aaaf01ab5e3deec2a9eeed18e) ([merge request](gitlab-org/security/gitlab!1685))
+- [Fix stored XSS vulnerability in Datadog settings form](gitlab-org/security/gitlab@269e5bf96b5e97c3b8e6f6b8b3f593d958de2ecb) ([merge request](gitlab-org/security/gitlab!1672))
+- [Inherit user external status while creating project bots](gitlab-org/security/gitlab@5bae4e53bd4c363270b2fc2e308b81d2a2a388a6) ([merge request](gitlab-org/security/gitlab!1666))
+- [Escape issue reference and title for Jira issues](gitlab-org/security/gitlab@0397f2b393d563559c49c39c0ba1d192d08a10d7) ([merge request](gitlab-org/security/gitlab!1663)) **GitLab Enterprise Edition**
+- [Require sign in for .keys endpoint on non-public instances](gitlab-org/security/gitlab@13a7f6001f663b3745159fa37b518ba4a43355bd) ([merge request](gitlab-org/security/gitlab!1659))
+- [Update Import/Export to use public email when mapping users](gitlab-org/security/gitlab@f3d1b800af55986cef83aeaf4df1312e3070f0c5) ([merge request](gitlab-org/security/gitlab!1654)) **GitLab Enterprise Edition**
+- [Only create jira connect NS subscriptions for admins](gitlab-org/security/gitlab@34bdcd45f24eaa051702834fb6c3568e45721004) ([merge request](gitlab-org/security/gitlab!1647))
+- [Prevent non-admins from configuring Jira connect app](gitlab-org/security/gitlab@4af692246224b1cd1e2fe3c6d0ac2613c0f8fe39) ([merge request](gitlab-org/security/gitlab!1643))
+
 ## 14.0.8 (2021-08-25)
 
 ### Fixed (1 change)

Gemfile

@@ -101,7 +101,7 @@ gem 'graphql', '~> 1.11.8'
 # TODO: remove app/views/graphiql/rails/editors/show.html.erb when https://github.com/rmosolgo/graphiql-rails/pull/71 is released:
 # https://gitlab.com/gitlab-org/gitlab/issues/31747
 gem 'graphiql-rails', '~> 1.4.10'
-gem 'apollo_upload_server', '~> 2.0.2'
+gem 'apollo_upload_server', '~> 2.1.0'
 gem 'graphql-docs', '~> 1.6.0', group: [:development, :test]
 gem 'graphlient', '~> 0.4.0' # Used by BulkImport feature (group::import)
 

Gemfile.lock

@@ -80,9 +80,9 @@ GEM
     aes_key_wrap (1.1.0)
     akismet (3.0.0)
     android_key_attestation (0.3.0)
-    apollo_upload_server (2.0.2)
+    apollo_upload_server (2.1.0)
+      actionpack (>= 4.2)
       graphql (>= 1.8)
-      rails (>= 4.2)
     asana (0.10.3)
       faraday (~> 1.0)
       faraday_middleware (~> 1.0)
@@ -1391,7 +1391,7 @@ DEPENDENCIES
   acts-as-taggable-on (~> 7.0)
   addressable (~> 2.8)
   akismet (~> 3.0)
-  apollo_upload_server (~> 2.0.2)
+  apollo_upload_server (~> 2.1.0)
   asana (~> 0.10.3)
   asciidoctor (~> 2.0.10)
   asciidoctor-include-ext (~> 0.3.1)

app/controllers/jira_connect/app_descriptor_controller.rb

@@ -47,7 +47,13 @@ class JiraConnect::AppDescriptorController < JiraConnect::ApplicationController
       postInstallPage: {
         key: 'gitlab-configuration',
         name: { value: 'GitLab Configuration' },
-        url: relative_to_base_path(jira_connect_subscriptions_path)
+        url: relative_to_base_path(jira_connect_subscriptions_path),
+        conditions: [
+          {
+            condition: 'user_is_admin',
+            invert: false
+          }
+        ]
       }
     }
 

app/controllers/jira_connect/application_controller.rb

@@ -38,12 +38,30 @@ class JiraConnect::ApplicationController < ApplicationController
   end
 
   def installation_from_jwt
-    return unless auth_token
-
     strong_memoize(:installation_from_jwt) do
+      next unless claims['iss']
+
+      JiraConnectInstallation.find_by_client_key(claims['iss'])
+    end
+  end
+
+  def claims
+    strong_memoize(:claims) do
+      next {} unless auth_token
+
       # Decode without verification to get `client_key` in `iss`
       payload, _ = Atlassian::Jwt.decode(auth_token, nil, false)
-      JiraConnectInstallation.find_by_client_key(payload['iss'])
+      payload
+    end
+  end
+
+  def jira_user
+    strong_memoize(:jira_user) do
+      next unless installation_from_jwt
+      next unless claims['sub']
+
+      # This only works for Jira Cloud installations.
+      installation_from_jwt.client.user_info(claims['sub'])
     end
   end
 

app/controllers/jira_connect/subscriptions_controller.rb

@@ -44,7 +44,9 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController
   def destroy
     subscription = current_jira_installation.subscriptions.find(params[:id])
 
-    if subscription.destroy
+    if !jira_user&.site_admin?
+      render json: { error: 'forbidden' }, status: :forbidden
+    elsif subscription.destroy
       render json: { success: true }
     else
       render json: { error: subscription.errors.full_messages.join(', ') }, status: :unprocessable_entity
@@ -54,7 +56,7 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController
   private
 
   def create_service
-    JiraConnectSubscriptions::CreateService.new(current_jira_installation, current_user, namespace_path: params['namespace_path'])
+    JiraConnectSubscriptions::CreateService.new(current_jira_installation, current_user, namespace_path: params['namespace_path'], jira_user: jira_user)
   end
 
   def allow_rendering_in_iframe

app/controllers/users_controller.rb

@@ -20,7 +20,7 @@ class UsersController < ApplicationController
 
   skip_before_action :authenticate_user!
   prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
-  before_action :user, except: [:exists, :ssh_keys]
+  before_action :user, except: [:exists]
   before_action :authorize_read_user_profile!,
                 only: [:calendar, :calendar_activities, :groups, :projects, :contributed, :starred, :snippets, :followers, :following]
 
@@ -44,12 +44,7 @@ class UsersController < ApplicationController
 
   # Get all keys of a user(params[:username]) in a text format
   # Helpful for sysadmins to put in respective servers
-  #
-  # Uses `UserFinder` rather than `find_routable!` because this endpoint should
-  # be publicly available regardless of instance visibility settings.
   def ssh_keys
-    user = UserFinder.new(params[:username]).find_by_username
-
     render plain: user.all_ssh_keys.join("\n")
   end
 

app/helpers/integrations_helper.rb

@@ -137,7 +137,7 @@ module IntegrationsHelper
   def jira_issue_breadcrumb_link(issue_reference)
     link_to '', { class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap' } do
       icon = image_tag image_path('illustrations/logos/jira.svg'), width: 15, height: 15, class: 'gl-mr-2'
-      [icon, issue_reference].join.html_safe
+      [icon, html_escape(issue_reference)].join.html_safe
     end
   end
 

app/models/design_management/design.rb

@@ -169,7 +169,7 @@ module DesignManagement
       @link_reference_pattern ||= begin
         path_segment = %r{issues/#{Gitlab::Regex.issue}/designs}
         ext = Regexp.new(Regexp.union(SAFE_IMAGE_EXT + DANGEROUS_IMAGE_EXT).source, Regexp::IGNORECASE)
-        valid_char = %r{[^/\s]} # any char that is not a forward slash or whitespace
+        valid_char = %r{[[:word:]\.\-\+]}
         filename_pattern = %r{
           (?<url_filename> #{valid_char}+ \. #{ext})
         }x

app/models/integrations/datadog.rb

@@ -8,7 +8,6 @@ module Integrations
 
     DEFAULT_DOMAIN = 'datadoghq.com'
     URL_TEMPLATE = 'https://webhooks-http-intake.logs.%{datadog_domain}/api/v2/webhook'
-    URL_TEMPLATE_API_KEYS = 'https://app.%{datadog_domain}/account/settings#api'
     URL_API_KEYS_DOCS = "https://docs.#{DEFAULT_DOMAIN}/account_management/api-app-keys/"
 
     SUPPORTED_EVENTS = %w[
@@ -90,7 +89,7 @@ module Integrations
           help: ERB::Util.html_escape(
             s_('DatadogIntegration|%{linkOpen}API key%{linkClose} used for authentication with Datadog.')
           ) % {
-            linkOpen: '<a href="%s" target="_blank" rel="noopener noreferrer">'.html_safe % api_keys_url,
+            linkOpen: %Q{<a href="#{URL_API_KEYS_DOCS}" target="_blank" rel="noopener noreferrer">}.html_safe,
             linkClose: '</a>'.html_safe
           },
           required: true
@@ -132,12 +131,6 @@ module Integrations
       url.to_s
     end
 
-    def api_keys_url
-      return URL_API_KEYS_DOCS unless datadog_site.presence
-
-      sprintf(URL_TEMPLATE_API_KEYS, datadog_domain: datadog_domain)
-    end
-
     def execute(data)
       object_kind = data[:object_kind]
       object_kind = 'job' if object_kind == 'build'

app/models/jira_connect_installation.rb

@@ -20,4 +20,8 @@ class JiraConnectInstallation < ApplicationRecord
         id: JiraConnectSubscription.for_project(project)
       })
   }
+
+  def client
+    Atlassian::JiraConnect::Client.new(base_url, shared_secret)
+  end
 end

app/services/groups/destroy_service.rb

@@ -29,14 +29,7 @@ module Groups
 
       group.chat_team&.remove_mattermost_team(current_user)
 
-      # If any other groups are shared with the group that is being destroyed,
-      # we should specifically trigger update of all project authorizations
-      # for users that are the direct members of this group.
-      # If not, the project authorization records of these users to projects within the shared groups
-      # will never be removed, causing inconsistencies with access permissions.
-      if any_other_groups_are_shared_with_this_group?
-        user_ids_for_project_authorizations_refresh = group.users_ids_of_direct_members
-      end
+      user_ids_for_project_authorizations_refresh = obtain_user_ids_for_project_authorizations_refresh
 
       group.destroy
 
@@ -52,9 +45,33 @@ module Groups
 
     private
 
-    def any_other_groups_are_shared_with_this_group?
+    def any_groups_shared_with_this_group?
       group.shared_group_links.any?
     end
+
+    def any_projects_shared_with_this_group?
+      group.project_group_links.any?
+    end
+
+    # Destroying a group automatically destroys all project authorizations directly
+    # associated with the group and descendents. However, project authorizations
+    # for projects and groups this group is shared with are not. Without a manual
+    # refresh, the project authorization records of these users to shared projects
+    # and projects within the shared groups will never be removed, causing
+    # inconsistencies with access permissions.
+    #
+    # This method retrieves the user IDs that need to be refreshed. If only
+    # groups are shared with this group, only direct members need to be refreshed.
+    # If projects are also shared with the group, direct members *and* shared
+    # members of other groups need to be refreshed.
+    # `Group#user_ids_for_project_authorizations` returns both direct and shared
+    # members' user IDs.
+    def obtain_user_ids_for_project_authorizations_refresh
+      return unless any_projects_shared_with_this_group? || any_groups_shared_with_this_group?
+      return group.user_ids_for_project_authorizations if any_projects_shared_with_this_group?
+
+      group.users_ids_of_direct_members
+    end
   end
 end
 

app/services/jira_connect_subscriptions/create_service.rb

@@ -5,8 +5,11 @@ module JiraConnectSubscriptions
     include Gitlab::Utils::StrongMemoize
     MERGE_REQUEST_SYNC_BATCH_SIZE = 20
     MERGE_REQUEST_SYNC_BATCH_DELAY = 1.minute.freeze
+    NOT_SITE_ADMIN = 'The Jira user is not a site administrator.'
 
     def execute
+      return error(NOT_SITE_ADMIN, 403) unless can_administer_jira?
+
       unless namespace && can?(current_user, :create_jira_connect_subscription, namespace)
         return error('Invalid namespace. Please make sure you have sufficient permissions', 401)
       end
@@ -16,6 +19,10 @@ module JiraConnectSubscriptions
 
     private
 
+    def can_administer_jira?
+      @params[:jira_user]&.site_admin?
+    end
+
     def create_subscription
       subscription = JiraConnectSubscription.new(installation: jira_connect_installation, namespace: namespace)
 

app/services/resource_access_tokens/create_service.rb

@@ -16,6 +16,8 @@ module ResourceAccessTokens
 
       return error(user.errors.full_messages.to_sentence) unless user.persisted?
 
+      user.update!(external: true) if current_user.external?
+
       access_level = params[:access_level] || Gitlab::Access::MAINTAINER
       member = create_membership(resource, user, access_level)
 

config/initializers/apollo_upload_server.rb

+# frozen_string_literal: true
+
+require 'apollo_upload_server'
+
+ApolloUploadServer::Middleware.strict_mode = true

db/post_migrate/20210811122206_update_external_project_bots.rb

+# frozen_string_literal: true
+
+class UpdateExternalProjectBots < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  class User < ActiveRecord::Base
+    self.table_name = 'users'
+  end
+
+  disable_ddl_transaction!
+
+  TMP_INDEX_NAME = 'tmp_idx_update_external_project_bots'
+
+  def up
+    add_concurrent_index('users', 'id', name: TMP_INDEX_NAME, where: 'external = true')
+
+    ids = ActiveRecord::Base.connection
+                            .execute("SELECT u.id FROM users u JOIN users u2 on u2.id = u.created_by_id WHERE u.user_type = 6 AND u2.external = true")
+                            .map { |result| result['id'] }
+
+    ids.each_slice(10) do |group|
+      UpdateExternalProjectBots::User.where(id: group).update_all(external: true)
+    end
+
+    remove_concurrent_index_by_name('users', TMP_INDEX_NAME)
+  end
+
+  def down
+    remove_concurrent_index_by_name('users', TMP_INDEX_NAME) if index_exists_by_name?('users', TMP_INDEX_NAME)
+
+    # This migration is irreversible
+  end
+end

db/schema_migrations/20210811122206

+f6f5e081672fb42adde980fa12f696f5d8fd11921ee52c1472b3d745bb11a5ff
\ No newline at end of file

doc/development/import_export.md

@@ -259,7 +259,7 @@ Only include the following attributes for the models specified:
 included_attributes:
   user:
     - :id
-    - :email
+    - :public_email
   # ...
 ```
 

doc/integration/jira/connect-app.md

@@ -6,6 +6,10 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 # GitLab.com for Jira Cloud app **(FREE)**
 
+NOTE:
+Only Jira users with administrator level access are able to install or configure
+the GitLab app for Jira Cloud.
+
 ## GitLab.com for Jira Cloud app **(FREE SAAS)**
 
 You can integrate GitLab.com and Jira Cloud using the
@@ -39,7 +43,8 @@ For a walkthrough of the integration with GitLab.com for Jira Cloud app, watch
    ![Sign in to GitLab.com in GitLab.com for Jira Cloud app](img/jira_dev_panel_setup_com_3_v13_9.png)
 1. Select **Add namespace** to open the list of available namespaces.
 
-1. Identify the namespace you want to link, and select **Link**.
+1. Identify the namespace you want to link, and select **Link**. Only Jira site
+   administrators are permitted to add or remove namespaces for an installation.
 
    ![Link namespace in GitLab.com for Jira Cloud app](img/jira_dev_panel_setup_com_4_v13_9.png)
 

doc/user/project/settings/import_export.md

@@ -44,7 +44,7 @@ Note the following:
 - Group members are exported as project members, as long as the user has
   a maintainer or administrator role in the group where the exported project lives.
 - Project members with the [Owner role](../../permissions.md) are imported as Maintainers.
-- Imported users can be mapped by their primary email on self-managed instances, if an administrative user (not an owner) does the import.
+- Imported users can be mapped by their public email on self-managed instances, if an administrative user (not an owner) does the import.
   Additionally, the user must be an existing member of the namespace, or the user can be added as a
 member of the project for contributions to be mapped.
   Otherwise, a supplementary comment is left to mention that the original author and

ee/app/views/projects/integrations/jira/issues/show.html.haml

 - add_to_breadcrumbs _('Jira Issues'), project_integrations_jira_issues_path(@project)
 - breadcrumb_title jira_issue_breadcrumb_link(@issue_json[:references][:relative])
-- page_title @issue_json[:title]
+- page_title html_escape(@issue_json[:title])
 
 .js-jira-issues-show-app{ data: jira_issues_show_data }

ee/spec/controllers/projects/integrations/jira/issues_controller_spec.rb

@@ -203,12 +203,12 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
       before do
         stub_licensed_features(jira_issues_integration: true)
 
-        expect_next_found_instance_of(Integrations::Jira) do |service|
-          expect(service).to receive(:find_issue).with('1', rendered_fields: true).and_return(jira_issue)
+        allow_next_found_instance_of(Integrations::Jira) do |service|
+          allow(service).to receive(:find_issue).with('1', rendered_fields: true).and_return(jira_issue)
         end
 
-        expect_next_instance_of(Integrations::JiraSerializers::IssueDetailSerializer) do |serializer|
-          expect(serializer).to receive(:represent).with(jira_issue, project: project).and_return(issue_json)
+        allow_next_instance_of(Integrations::JiraSerializers::IssueDetailSerializer) do |serializer|
+          allow(serializer).to receive(:represent).with(jira_issue, project: project).and_return(issue_json)
         end
       end
 
@@ -225,6 +225,21 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
 
         expect(json_response).to eq(issue_json)
       end
+
+      context 'when the JSON fetched from Jira contains HTML' do
+        let(:payload) { "<script>alert('XSS')</script>" }
+        let(:issue_json) { { title: payload, references: { relative: payload } } }
+
+        render_views
+
+        it 'escapes the HTML in issue titles and references' do
+          get :show, params: { namespace_id: project.namespace, project_id: project, id: 1 }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response.body).not_to include(payload)
+          expect(response.body).to include(html_escape(payload))
+        end
+      end
     end
   end
 end

ee/spec/fixtures/lib/gitlab/import_export/complex/project.json

@@ -21,7 +21,7 @@
       "requested_at": null,
       "user": {
         "id": 16,
-        "email": "bernard_willms@gitlabexample.com",
+        "public_email": "bernard_willms@gitlabexample.com",
         "username": "bernard_willms"
       }
     }

ee/spec/fixtures/lib/gitlab/import_export/group_exports/light/group.json

@@ -204,7 +204,7 @@
       "override": false,
       "user": {
         "id": 42,
-        "email": "moriah@collinsmurphy.com",
+        "public_email": "moriah@collinsmurphy.com",
         "username": "reported_user_15"
       }
     }
@@ -433,7 +433,7 @@
           },
           "author": {
             "id": 1,
-            "email": "",
+            "public_email": "",
             "created_at": "2021-04-14T14:42:15.926Z",
             "updated_at": "2021-06-08T16:32:26.109Z",
             "name": "Administrator"

ee/spec/fixtures/lib/gitlab/import_export/group_exports/light/tree/groups/761/members.ndjson

-{"id":13766,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":42,"notification_level":3,"created_at":"2019-11-20T17:04:36.184Z","updated_at":"2019-11-20T17:04:36.184Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":42,"email":"moriah@collinsmurphy.com","username":"reported_user_15"}}
+{"id":13766,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":42,"notification_level":3,"created_at":"2019-11-20T17:04:36.184Z","updated_at":"2019-11-20T17:04:36.184Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":42,"public_email":"moriah@collinsmurphy.com","username":"reported_user_15"}}

lib/atlassian/jira_connect/client.rb

@@ -30,8 +30,21 @@ module Atlassian
         responses.compact
       end
 
+      def user_info(account_id)
+        r = get('/rest/api/3/user', { accountId: account_id, expand: 'groups' })
+
+        JiraUser.new(r.parsed_response) if r.code == 200
+      end
+
       private
 
+      def get(path, query_params)
+        uri = URI.join(@base_uri, path)
+        uri.query = URI.encode_www_form(query_params)
+
+        self.class.get(uri, headers: headers(uri, 'GET'))
+      end
+
       def store_ff_info(project:, feature_flags:, **opts)
         items = feature_flags.map { |flag| ::Atlassian::JiraConnect::Serializers::FeatureFlagEntity.represent(flag, opts) }
         items.reject! { |item| item.issue_keys.empty? }
@@ -99,10 +112,11 @@ module Atlassian
         self.class.post(uri, headers: headers(uri), body: metadata.merge(payload).to_json)
       end
 
-      def headers(uri)
+      def headers(uri, http_method = 'POST')
         {
-          'Authorization' => "JWT #{jwt_token('POST', uri)}",
-          'Content-Type' => 'application/json'
+          'Authorization' => "JWT #{jwt_token(http_method, uri)}",
+          'Content-Type' => 'application/json',
+          'Accept' => 'application/json'
         }
       end
 

lib/atlassian/jira_connect/jira_user.rb

+# frozen_string_literal: true
+
+module Atlassian
+  module JiraConnect
+    class JiraUser
+      def initialize(data)
+        @data = data
+      end
+
+      def site_admin?
+        groups = @data.dig('groups', 'items')
+        return false unless groups
+
+        groups.any? { |g| g['name'] == 'site-admins' }
+      end
+    end
+  end
+end

lib/gitlab/import_export/group/import_export.yml

@@ -20,7 +20,7 @@ tree:
 included_attributes:
   user:
     - :id
-    - :email
+    - :public_email
     - :username
   author:
     - :name

lib/gitlab/import_export/group/legacy_import_export.yml

@@ -20,7 +20,7 @@ tree:
 included_attributes:
   user:
     - :id
-    - :email
+    - :public_email
     - :username
   author:
     - :name

lib/gitlab/import_export/members_mapper.rb

@@ -19,7 +19,8 @@ module Gitlab
             @exported_members.inject(missing_keys_tracking_hash) do |hash, member|
               if member['user']
                 old_user_id = member['user']['id']
-                existing_user = User.find_by(find_user_query(member))
+                old_user_email = member.dig('user', 'public_email') || member.dig('user', 'email')
+                existing_user = User.find_by(find_user_query(old_user_email)) if old_user_email
                 hash[old_user_id] = existing_user.id if existing_user && add_team_member(member, existing_user)
               else
                 add_team_member(member)
@@ -94,8 +95,8 @@ module Gitlab
                                                      relation_class: relation_class)
       end
 
-      def find_user_query(member)
-        user_arel[:email].eq(member['user']['email'])
+      def find_user_query(email)
+        user_arel[:email].eq(email)
       end
 
       def user_arel

lib/gitlab/import_export/project/import_export.yml

@@ -115,7 +115,7 @@ tree:
 included_attributes:
   user:
     - :id
-    - :email
+    - :public_email
     - :username
   author:
     - :name

lib/gitlab/markdown_cache.rb

@@ -11,7 +11,7 @@ module Gitlab
     # this if the change to the renderer output is a new feature or a
     # minor bug fix.
     # See: https://gitlab.com/gitlab-org/gitlab/-/issues/330313
-    CACHE_COMMONMARK_VERSION        = 28
+    CACHE_COMMONMARK_VERSION        = 29
     CACHE_COMMONMARK_VERSION_START  = 10
 
     BaseError = Class.new(StandardError)

lib/gitlab/middleware/multipart.rb

@@ -176,7 +176,7 @@ module Gitlab
         ::Gitlab::Middleware::Multipart::Handler.new(env, message).with_open_files do
           @app.call(env)
         end
-      rescue UploadedFile::InvalidPathError => e
+      rescue UploadedFile::InvalidPathError, ApolloUploadServer::GraphQLDataBuilder::OutOfBounds => e
         [400, { 'Content-Type' => 'text/plain' }, [e.message]]
       end
     end

spec/controllers/jira_connect/app_descriptor_controller_spec.rb

@@ -54,7 +54,10 @@ RSpec.describe JiraConnect::AppDescriptorController do
         postInstallPage: {
           key: 'gitlab-configuration',
           name: { value: 'GitLab Configuration' },
-          url: '/subscriptions'
+          url: '/subscriptions',
+          conditions: contain_exactly(
+            a_hash_including(condition: 'user_is_admin', invert: false)
+          )
         },
         jiraDevelopmentTool: {
           actions: {

spec/controllers/jira_connect/subscriptions_controller_spec.rb

@@ -102,11 +102,17 @@ RSpec.describe JiraConnect::SubscriptionsController do
     end
 
     context 'with valid JWT' do
-      let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) }
+      let(:claims) { { iss: installation.client_key, sub: 1234 } }
+      let(:jwt) { Atlassian::Jwt.encode(claims, installation.shared_secret) }
+      let(:jira_user) { { 'groups' => { 'items' => [{ 'name' => jira_group_name }] } } }
+      let(:jira_group_name) { 'site-admins' }
 
       context 'signed in to GitLab' do
         before do
           sign_in(user)
+          WebMock
+            .stub_request(:get, "#{installation.base_url}/rest/api/3/user?accountId=1234&expand=groups")
+            .to_return(body: jira_user.to_json, status: 200, headers: { 'Content-Type' => 'application/json' })
         end
 
         context 'dev panel integration is available' do
@@ -120,6 +126,16 @@ RSpec.describe JiraConnect::SubscriptionsController do
             expect(response).to have_gitlab_http_status(:ok)
           end
         end
+
+        context 'when the Jira user is not a site-admin' do
+          let(:jira_group_name) { 'some-other-group' }
+
+          it 'returns forbidden' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
+        end
       end
 
       context 'not signed in to GitLab' do
@@ -134,8 +150,14 @@ RSpec.describe JiraConnect::SubscriptionsController do
 
   describe '#destroy' do
     let(:subscription) { create(:jira_connect_subscription, installation: installation) }
+    let(:jira_user) { { 'groups' => { 'items' => [{ 'name' => jira_group_name }] } } }
+    let(:jira_group_name) { 'site-admins' }
 
     before do
+      WebMock
+        .stub_request(:get, "#{installation.base_url}/rest/api/3/user?accountId=1234&expand=groups")
+        .to_return(body: jira_user.to_json, status: 200, headers: { 'Content-Type' => 'application/json' })
+
       delete :destroy, params: { jwt: jwt, id: subscription.id }
     end
 
@@ -148,12 +170,23 @@ RSpec.describe JiraConnect::SubscriptionsController do
     end
 
     context 'with valid JWT' do
-      let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) }
+      let(:claims) { { iss: installation.client_key, sub: 1234 } }
+      let(:jwt) { Atlassian::Jwt.encode(claims, installation.shared_secret) }
 
       it 'deletes the subscription' do
         expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound
         expect(response).to have_gitlab_http_status(:ok)
       end
+
+      context 'when the Jira user is not a site admin' do
+        let(:jira_group_name) { 'some-other-group' }
+
+        it 'does not delete the subscription' do
+          expect(response).to have_gitlab_http_status(:forbidden)
+
+          expect { subscription.reload }.not_to raise_error
+        end
+      end
     end
   end
 end

spec/features/file_uploads/graphql_add_design_spec.rb

@@ -19,8 +19,18 @@ RSpec.describe 'Upload a design through graphQL', :js do
   let_it_be(:user) { create(:user, :admin) }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
   let_it_be(:design) { create(:design) }
-  let_it_be(:operations) { { "operationName": "uploadDesign", "variables": { "files": [], "projectPath": design.project.full_path, "iid": design.issue.iid }, "query": query }.to_json }
   let_it_be(:map) { { "1": ["variables.files.0"] }.to_json }
+  let_it_be(:operations) do
+    {
+      "operationName": "uploadDesign",
+      "variables": {
+        "files": [nil],
+        "projectPath": design.project.full_path,
+        "iid": design.issue.iid
+      },
+      "query": query
+    }.to_json
+  end
 
   let(:url) { capybara_url("/api/graphql?private_token=#{personal_access_token.token}") }
   let(:file) { fixture_file_upload('spec/fixtures/dk.png') }

spec/fixtures/lib/gitlab/import_export/complex/project.json

@@ -2398,7 +2398,7 @@
       "requested_at": null,
       "user": {
         "id": 16,
-        "email": "bernard_willms@gitlabexample.com",
+        "public_email": "bernard_willms@gitlabexample.com",
         "username": "bernard_willms"
       }
     },
@@ -2418,7 +2418,7 @@
       "requested_at": null,
       "user": {
         "id": 6,
-        "email": "saul_will@gitlabexample.com",
+        "public_email": "saul_will@gitlabexample.com",
         "username": "saul_will"
       }
     },
@@ -2438,7 +2438,7 @@
       "requested_at": null,
       "user": {
         "id": 15,
-        "email": "breanna_sanford@wolf.com",
+        "public_email": "breanna_sanford@wolf.com",
         "username": "emmet.schamberger"
       }
     },
@@ -2458,7 +2458,7 @@
       "requested_at": null,
       "user": {
         "id": 26,
-        "email": "user4@example.com",
+        "public_email": "user4@example.com",
         "username": "user4"
       }
     }

spec/fixtures/lib/gitlab/import_export/complex/tree/project/project_members.ndjson

-{"id":36,"access_level":40,"source_id":5,"source_type":"Project","user_id":16,"notification_level":3,"created_at":"2016-06-14T15:02:03.834Z","updated_at":"2016-06-14T15:02:03.834Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"user":{"id":16,"email":"bernard_willms@gitlabexample.com","username":"bernard_willms"}}
-{"id":35,"access_level":10,"source_id":5,"source_type":"Project","user_id":6,"notification_level":3,"created_at":"2016-06-14T15:02:03.811Z","updated_at":"2016-06-14T15:02:03.811Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"user":{"id":6,"email":"saul_will@gitlabexample.com","username":"saul_will"}}
-{"id":34,"access_level":20,"source_id":5,"source_type":"Project","user_id":15,"notification_level":3,"created_at":"2016-06-14T15:02:03.776Z","updated_at":"2016-06-14T15:02:03.776Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"user":{"id":15,"email":"breanna_sanford@wolf.com","username":"emmet.schamberger"}}
-{"id":33,"access_level":20,"source_id":5,"source_type":"Project","user_id":26,"notification_level":3,"created_at":"2016-06-14T15:02:03.742Z","updated_at":"2016-06-14T15:02:03.742Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"user":{"id":26,"email":"user4@example.com","username":"user4"}}
+{"id":36,"access_level":40,"source_id":5,"source_type":"Project","user_id":16,"notification_level":3,"created_at":"2016-06-14T15:02:03.834Z","updated_at":"2016-06-14T15:02:03.834Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"user":{"id":16,"public_email":"bernard_willms@gitlabexample.com","username":"bernard_willms"}}
+{"id":35,"access_level":10,"source_id":5,"source_type":"Project","user_id":6,"notification_level":3,"created_at":"2016-06-14T15:02:03.811Z","updated_at":"2016-06-14T15:02:03.811Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"user":{"id":6,"public_email":"saul_will@gitlabexample.com","username":"saul_will"}}
+{"id":34,"access_level":20,"source_id":5,"source_type":"Project","user_id":15,"notification_level":3,"created_at":"2016-06-14T15:02:03.776Z","updated_at":"2016-06-14T15:02:03.776Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"user":{"id":15,"public_email":"breanna_sanford@wolf.com","username":"emmet.schamberger"}}
+{"id":33,"access_level":20,"source_id":5,"source_type":"Project","user_id":26,"notification_level":3,"created_at":"2016-06-14T15:02:03.742Z","updated_at":"2016-06-14T15:02:03.742Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"user":{"id":26,"public_email":"user4@example.com","username":"user4"}}

spec/fixtures/lib/gitlab/import_export/designs/project.json

@@ -425,7 +425,7 @@
          "override":false,
          "user":{
             "id":1,
-            "email":"admin@example.com",
+            "public_email":"admin@example.com",
             "username":"root"
          }
       },
@@ -448,7 +448,7 @@
          "override":false,
          "user":{
             "id":2,
-            "email":"user_2@gitlabexample.com",
+            "public_email":"user_2@gitlabexample.com",
             "username":"user_2"
          }
       }

spec/fixtures/lib/gitlab/import_export/group_exports/complex/group.json

@@ -377,7 +377,7 @@
       "override": false,
       "user": {
         "id": 42,
-        "email": "moriah@collinsmurphy.com",
+        "public_email": "moriah@collinsmurphy.com",
         "username": "reported_user_15"
       }
     },
@@ -400,7 +400,7 @@
       "override": false,
       "user": {
         "id": 271,
-        "email": "garret@connellystark.ca",
+        "public_email": "garret@connellystark.ca",
         "username": "charlesetta"
       }
     },
@@ -423,7 +423,7 @@
       "override": false,
       "user": {
         "id": 206,
-        "email": "gwendolyn_robel@gitlabexample.com",
+        "public_email": "gwendolyn_robel@gitlabexample.com",
         "username": "gwendolyn_robel"
       }
     },
@@ -446,7 +446,7 @@
       "override": false,
       "user": {
         "id": 39,
-        "email": "alexis_berge@kerlukeklein.us",
+        "public_email": "alexis_berge@kerlukeklein.us",
         "username": "reported_user_12"
       }
     },
@@ -469,7 +469,7 @@
       "override": false,
       "user": {
         "id": 1624,
-        "email": "adriene.mcclure@gitlabexample.com",
+        "public_email": "adriene.mcclure@gitlabexample.com",
         "username": "adriene.mcclure"
       }
     },
@@ -492,7 +492,7 @@
       "override": false,
       "user": {
         "id": 1,
-        "email": "admin@example.com",
+        "public_email": "admin@example.com",
         "username": "root"
       }
     }
@@ -1338,7 +1338,7 @@
           "override": false,
           "user": {
             "id": 1087,
-            "email": "paige@blanda.info",
+            "public_email": "paige@blanda.info",
             "username": "billi_auer"
           }
         },
@@ -1361,7 +1361,7 @@
           "override": false,
           "user": {
             "id": 171,
-            "email": "heidi@bosco.co.uk",
+            "public_email": "heidi@bosco.co.uk",
             "username": "gerard.cruickshank"
           }
         },
@@ -1384,7 +1384,7 @@
           "override": false,
           "user": {
             "id": 1157,
-            "email": "larisa.bruen@carroll.biz",
+            "public_email": "larisa.bruen@carroll.biz",
             "username": "milagros.reynolds"
           }
         },
@@ -1407,7 +1407,7 @@
           "override": false,
           "user": {
             "id": 14,
-            "email": "madlyn_kovacek@wiza.ca",
+            "public_email": "madlyn_kovacek@wiza.ca",
             "username": "monique.gusikowski"
           }
         },
@@ -1430,7 +1430,7 @@
           "override": false,
           "user": {
             "id": 1167,
-            "email": "mirella@koepp.ca",
+            "public_email": "mirella@koepp.ca",
             "username": "eileen"
           }
         },
@@ -1453,7 +1453,7 @@
           "override": false,
           "user": {
             "id": 1,
-            "email": "admin@example.com",
+            "public_email": "admin@example.com",
             "username": "root"
           }
         }
@@ -1909,7 +1909,7 @@
           "override": false,
           "user": {
             "id": 1533,
-            "email": "jose@cassin.ca",
+            "public_email": "jose@cassin.ca",
             "username": "buster"
           }
         },
@@ -1932,7 +1932,7 @@
           "override": false,
           "user": {
             "id": 1586,
-            "email": "carie@gleichner.us",
+            "public_email": "carie@gleichner.us",
             "username": "dominque"
           }
         },
@@ -1955,7 +1955,7 @@
           "override": false,
           "user": {
             "id": 190,
-            "email": "delois@funk.biz",
+            "public_email": "delois@funk.biz",
             "username": "kittie"
           }
         },
@@ -1978,7 +1978,7 @@
           "override": false,
           "user": {
             "id": 254,
-            "email": "tyra.lowe@whitemckenzie.co.uk",
+            "public_email": "tyra.lowe@whitemckenzie.co.uk",
             "username": "kassie"
           }
         },
@@ -2001,7 +2001,7 @@
           "override": false,
           "user": {
             "id": 503,
-            "email": "tyesha.brakus@bruen.ca",
+            "public_email": "tyesha.brakus@bruen.ca",
             "username": "charise"
           }
         },
@@ -2024,7 +2024,7 @@
           "override": false,
           "user": {
             "id": 1,
-            "email": "admin@example.com",
+            "public_email": "admin@example.com",
             "username": "root"
           }
         }

spec/fixtures/lib/gitlab/import_export/group_exports/complex/tree/groups/4351/members.ndjson

-{"id":13766,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":42,"notification_level":3,"created_at":"2019-11-20T17:04:36.184Z","updated_at":"2019-11-20T17:04:36.184Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":42,"email":"moriah@collinsmurphy.com","username":"reported_user_15"}}
-{"id":13765,"access_level":40,"source_id":4351,"source_type":"Namespace","user_id":271,"notification_level":3,"created_at":"2019-11-20T17:04:36.044Z","updated_at":"2019-11-20T17:04:36.044Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":271,"email":"garret@connellystark.ca","username":"charlesetta"}}
-{"id":13764,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":206,"notification_level":3,"created_at":"2019-11-20T17:04:35.840Z","updated_at":"2019-11-20T17:04:35.840Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":206,"email":"gwendolyn_robel@gitlabexample.com","username":"gwendolyn_robel"}}
-{"id":13763,"access_level":10,"source_id":4351,"source_type":"Namespace","user_id":39,"notification_level":3,"created_at":"2019-11-20T17:04:35.704Z","updated_at":"2019-11-20T17:04:35.704Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":39,"email":"alexis_berge@kerlukeklein.us","username":"reported_user_12"}}
-{"id":13762,"access_level":20,"source_id":4351,"source_type":"Namespace","user_id":1624,"notification_level":3,"created_at":"2019-11-20T17:04:35.566Z","updated_at":"2019-11-20T17:04:35.566Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1624,"email":"adriene.mcclure@gitlabexample.com","username":"adriene.mcclure"}}
-{"id":12920,"access_level":50,"source_id":4351,"source_type":"Namespace","user_id":1,"notification_level":3,"created_at":"2019-11-20T17:01:53.505Z","updated_at":"2019-11-20T17:01:53.505Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1,"email":"admin@example.com","username":"root"}}
+{"id":13766,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":42,"notification_level":3,"created_at":"2019-11-20T17:04:36.184Z","updated_at":"2019-11-20T17:04:36.184Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":42,"public_email":"moriah@collinsmurphy.com","username":"reported_user_15"}}
+{"id":13765,"access_level":40,"source_id":4351,"source_type":"Namespace","user_id":271,"notification_level":3,"created_at":"2019-11-20T17:04:36.044Z","updated_at":"2019-11-20T17:04:36.044Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":271,"public_email":"garret@connellystark.ca","username":"charlesetta"}}
+{"id":13764,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":206,"notification_level":3,"created_at":"2019-11-20T17:04:35.840Z","updated_at":"2019-11-20T17:04:35.840Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":206,"public_email":"gwendolyn_robel@gitlabexample.com","username":"gwendolyn_robel"}}
+{"id":13763,"access_level":10,"source_id":4351,"source_type":"Namespace","user_id":39,"notification_level":3,"created_at":"2019-11-20T17:04:35.704Z","updated_at":"2019-11-20T17:04:35.704Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":39,"public_email":"alexis_berge@kerlukeklein.us","username":"reported_user_12"}}
+{"id":13762,"access_level":20,"source_id":4351,"source_type":"Namespace","user_id":1624,"notification_level":3,"created_at":"2019-11-20T17:04:35.566Z","updated_at":"2019-11-20T17:04:35.566Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1624,"public_email":"adriene.mcclure@gitlabexample.com","username":"adriene.mcclure"}}
+{"id":12920,"access_level":50,"source_id":4351,"source_type":"Namespace","user_id":1,"notification_level":3,"created_at":"2019-11-20T17:01:53.505Z","updated_at":"2019-11-20T17:01:53.505Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1,"public_email":"admin@example.com","username":"root"}}

spec/fixtures/lib/gitlab/import_export/group_exports/complex/tree/groups/4352/members.ndjson

-{"id":13771,"access_level":30,"source_id":4352,"source_type":"Namespace","user_id":1087,"notification_level":3,"created_at":"2019-11-20T17:04:36.968Z","updated_at":"2019-11-20T17:04:36.968Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1087,"email":"paige@blanda.info","username":"billi_auer"}}
-{"id":13770,"access_level":20,"source_id":4352,"source_type":"Namespace","user_id":171,"notification_level":3,"created_at":"2019-11-20T17:04:36.821Z","updated_at":"2019-11-20T17:04:36.821Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":171,"email":"heidi@bosco.co.uk","username":"gerard.cruickshank"}}
-{"id":13769,"access_level":30,"source_id":4352,"source_type":"Namespace","user_id":1157,"notification_level":3,"created_at":"2019-11-20T17:04:36.606Z","updated_at":"2019-11-20T17:04:36.606Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1157,"email":"larisa.bruen@carroll.biz","username":"milagros.reynolds"}}
-{"id":13768,"access_level":40,"source_id":4352,"source_type":"Namespace","user_id":14,"notification_level":3,"created_at":"2019-11-20T17:04:36.465Z","updated_at":"2019-11-20T17:04:36.465Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":14,"email":"madlyn_kovacek@wiza.ca","username":"monique.gusikowski"}}
-{"id":13767,"access_level":10,"source_id":4352,"source_type":"Namespace","user_id":1167,"notification_level":3,"created_at":"2019-11-20T17:04:36.324Z","updated_at":"2019-11-20T17:04:36.324Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1167,"email":"mirella@koepp.ca","username":"eileen"}}
-{"id":12921,"access_level":50,"source_id":4352,"source_type":"Namespace","user_id":1,"notification_level":3,"created_at":"2019-11-20T17:01:53.953Z","updated_at":"2019-11-20T17:01:53.953Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1,"email":"admin@example.com","username":"root"}}
+{"id":13771,"access_level":30,"source_id":4352,"source_type":"Namespace","user_id":1087,"notification_level":3,"created_at":"2019-11-20T17:04:36.968Z","updated_at":"2019-11-20T17:04:36.968Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1087,"public_email":"paige@blanda.info","username":"billi_auer"}}
+{"id":13770,"access_level":20,"source_id":4352,"source_type":"Namespace","user_id":171,"notification_level":3,"created_at":"2019-11-20T17:04:36.821Z","updated_at":"2019-11-20T17:04:36.821Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":171,"public_email":"heidi@bosco.co.uk","username":"gerard.cruickshank"}}
+{"id":13769,"access_level":30,"source_id":4352,"source_type":"Namespace","user_id":1157,"notification_level":3,"created_at":"2019-11-20T17:04:36.606Z","updated_at":"2019-11-20T17:04:36.606Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1157,"public_email":"larisa.bruen@carroll.biz","username":"milagros.reynolds"}}
+{"id":13768,"access_level":40,"source_id":4352,"source_type":"Namespace","user_id":14,"notification_level":3,"created_at":"2019-11-20T17:04:36.465Z","updated_at":"2019-11-20T17:04:36.465Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":14,"public_email":"madlyn_kovacek@wiza.ca","username":"monique.gusikowski"}}
+{"id":13767,"access_level":10,"source_id":4352,"source_type":"Namespace","user_id":1167,"notification_level":3,"created_at":"2019-11-20T17:04:36.324Z","updated_at":"2019-11-20T17:04:36.324Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1167,"public_email":"mirella@koepp.ca","username":"eileen"}}
+{"id":12921,"access_level":50,"source_id":4352,"source_type":"Namespace","user_id":1,"notification_level":3,"created_at":"2019-11-20T17:01:53.953Z","updated_at":"2019-11-20T17:01:53.953Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1,"public_email":"admin@example.com","username":"root"}}

spec/fixtures/lib/gitlab/import_export/group_exports/complex/tree/groups/4353/members.ndjson

-{"id":13786,"access_level":30,"source_id":4355,"source_type":"Namespace","user_id":1533,"notification_level":3,"created_at":"2019-11-20T17:04:39.405Z","updated_at":"2019-11-20T17:04:39.405Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1533,"email":"jose@cassin.ca","username":"buster"}}
-{"id":13785,"access_level":10,"source_id":4355,"source_type":"Namespace","user_id":1586,"notification_level":3,"created_at":"2019-11-20T17:04:39.269Z","updated_at":"2019-11-20T17:04:39.269Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1586,"email":"carie@gleichner.us","username":"dominque"}}
-{"id":13784,"access_level":30,"source_id":4355,"source_type":"Namespace","user_id":190,"notification_level":3,"created_at":"2019-11-20T17:04:39.127Z","updated_at":"2019-11-20T17:04:39.127Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":190,"email":"delois@funk.biz","username":"kittie"}}
-{"id":13783,"access_level":20,"source_id":4355,"source_type":"Namespace","user_id":254,"notification_level":3,"created_at":"2019-11-20T17:04:38.971Z","updated_at":"2019-11-20T17:04:38.971Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":254,"email":"tyra.lowe@whitemckenzie.co.uk","username":"kassie"}}
-{"id":13782,"access_level":40,"source_id":4355,"source_type":"Namespace","user_id":503,"notification_level":3,"created_at":"2019-11-20T17:04:38.743Z","updated_at":"2019-11-20T17:04:38.743Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":503,"email":"tyesha.brakus@bruen.ca","username":"charise"}}
-{"id":12924,"access_level":50,"source_id":4355,"source_type":"Namespace","user_id":1,"notification_level":3,"created_at":"2019-11-20T17:01:54.145Z","updated_at":"2019-11-20T17:01:54.145Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1,"email":"admin@example.com","username":"root"}}
+{"id":13786,"access_level":30,"source_id":4355,"source_type":"Namespace","user_id":1533,"notification_level":3,"created_at":"2019-11-20T17:04:39.405Z","updated_at":"2019-11-20T17:04:39.405Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1533,"public_email":"jose@cassin.ca","username":"buster"}}
+{"id":13785,"access_level":10,"source_id":4355,"source_type":"Namespace","user_id":1586,"notification_level":3,"created_at":"2019-11-20T17:04:39.269Z","updated_at":"2019-11-20T17:04:39.269Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1586,"public_email":"carie@gleichner.us","username":"dominque"}}
+{"id":13784,"access_level":30,"source_id":4355,"source_type":"Namespace","user_id":190,"notification_level":3,"created_at":"2019-11-20T17:04:39.127Z","updated_at":"2019-11-20T17:04:39.127Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":190,"public_email":"delois@funk.biz","username":"kittie"}}
+{"id":13783,"access_level":20,"source_id":4355,"source_type":"Namespace","user_id":254,"notification_level":3,"created_at":"2019-11-20T17:04:38.971Z","updated_at":"2019-11-20T17:04:38.971Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":254,"public_email":"tyra.lowe@whitemckenzie.co.uk","username":"kassie"}}
+{"id":13782,"access_level":40,"source_id":4355,"source_type":"Namespace","user_id":503,"notification_level":3,"created_at":"2019-11-20T17:04:38.743Z","updated_at":"2019-11-20T17:04:38.743Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":503,"public_email":"tyesha.brakus@bruen.ca","username":"charise"}}
+{"id":12924,"access_level":50,"source_id":4355,"source_type":"Namespace","user_id":1,"notification_level":3,"created_at":"2019-11-20T17:01:54.145Z","updated_at":"2019-11-20T17:01:54.145Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1,"public_email":"admin@example.com","username":"root"}}

spec/fixtures/lib/gitlab/import_export/group_exports/no_children/group.json

@@ -299,7 +299,7 @@
       "override": false,
       "user": {
         "id": 42,
-        "email": "moriah@collinsmurphy.com",
+        "public_email": "moriah@collinsmurphy.com",
         "username": "reported_user_15"
       }
     },
@@ -322,7 +322,7 @@
       "override": false,
       "user": {
         "id": 271,
-        "email": "garret@connellystark.ca",
+        "public_email": "garret@connellystark.ca",
         "username": "charlesetta"
       }
     },
@@ -345,7 +345,7 @@
       "override": false,
       "user": {
         "id": 206,
-        "email": "margaret.bergnaum@reynolds.us",
+        "public_email": "margaret.bergnaum@reynolds.us",
         "username": "gwendolyn_robel"
       }
     },
@@ -368,7 +368,7 @@
       "override": false,
       "user": {
         "id": 39,
-        "email": "alexis_berge@kerlukeklein.us",
+        "public_email": "alexis_berge@kerlukeklein.us",
         "username": "reported_user_12"
       }
     },
@@ -391,7 +391,7 @@
       "override": false,
       "user": {
         "id": 1624,
-        "email": "nakesha.herzog@powlowski.com",
+        "public_email": "nakesha.herzog@powlowski.com",
         "username": "adriene.mcclure"
       }
     },
@@ -414,7 +414,7 @@
       "override": false,
       "user": {
         "id": 1,
-        "email": "admin@example.com",
+        "public_email": "admin@example.com",
         "username": "root"
       }
     }

spec/fixtures/lib/gitlab/import_export/group_exports/no_children/tree/groups/4353/members.ndjson

-{"id":13766,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":42,"notification_level":3,"created_at":"2019-11-20T17:04:36.184Z","updated_at":"2019-11-20T17:04:36.184Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":42,"email":"moriah@collinsmurphy.com","username":"reported_user_15"}}
-{"id":13765,"access_level":40,"source_id":4351,"source_type":"Namespace","user_id":271,"notification_level":3,"created_at":"2019-11-20T17:04:36.044Z","updated_at":"2019-11-20T17:04:36.044Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":271,"email":"garret@connellystark.ca","username":"charlesetta"}}
-{"id":13764,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":206,"notification_level":3,"created_at":"2019-11-20T17:04:35.840Z","updated_at":"2019-11-20T17:04:35.840Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":206,"email":"margaret.bergnaum@reynolds.us","username":"gwendolyn_robel"}}
-{"id":13763,"access_level":10,"source_id":4351,"source_type":"Namespace","user_id":39,"notification_level":3,"created_at":"2019-11-20T17:04:35.704Z","updated_at":"2019-11-20T17:04:35.704Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":39,"email":"alexis_berge@kerlukeklein.us","username":"reported_user_12"}}
-{"id":13762,"access_level":20,"source_id":4351,"source_type":"Namespace","user_id":1624,"notification_level":3,"created_at":"2019-11-20T17:04:35.566Z","updated_at":"2019-11-20T17:04:35.566Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1624,"email":"nakesha.herzog@powlowski.com","username":"adriene.mcclure"}}
-{"id":12920,"access_level":50,"source_id":4351,"source_type":"Namespace","user_id":1,"notification_level":3,"created_at":"2019-11-20T17:01:53.505Z","updated_at":"2019-11-20T17:01:53.505Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1,"email":"admin@example.com","username":"root"}}
+{"id":13766,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":42,"notification_level":3,"created_at":"2019-11-20T17:04:36.184Z","updated_at":"2019-11-20T17:04:36.184Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":42,"public_email":"moriah@collinsmurphy.com","username":"reported_user_15"}}
+{"id":13765,"access_level":40,"source_id":4351,"source_type":"Namespace","user_id":271,"notification_level":3,"created_at":"2019-11-20T17:04:36.044Z","updated_at":"2019-11-20T17:04:36.044Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":271,"public_email":"garret@connellystark.ca","username":"charlesetta"}}
+{"id":13764,"access_level":30,"source_id":4351,"source_type":"Namespace","user_id":206,"notification_level":3,"created_at":"2019-11-20T17:04:35.840Z","updated_at":"2019-11-20T17:04:35.840Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":206,"public_email":"margaret.bergnaum@reynolds.us","username":"gwendolyn_robel"}}
+{"id":13763,"access_level":10,"source_id":4351,"source_type":"Namespace","user_id":39,"notification_level":3,"created_at":"2019-11-20T17:04:35.704Z","updated_at":"2019-11-20T17:04:35.704Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":39,"public_email":"alexis_berge@kerlukeklein.us","username":"reported_user_12"}}
+{"id":13762,"access_level":20,"source_id":4351,"source_type":"Namespace","user_id":1624,"notification_level":3,"created_at":"2019-11-20T17:04:35.566Z","updated_at":"2019-11-20T17:04:35.566Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1624,"public_email":"nakesha.herzog@powlowski.com","username":"adriene.mcclure"}}
+{"id":12920,"access_level":50,"source_id":4351,"source_type":"Namespace","user_id":1,"notification_level":3,"created_at":"2019-11-20T17:01:53.505Z","updated_at":"2019-11-20T17:01:53.505Z","created_by_id":null,"invite_email":null,"invite_token":null,"invite_accepted_at":null,"requested_at":null,"expires_at":null,"ldap":false,"override":false,"user":{"id":1,"public_email":"admin@example.com","username":"root"}}

spec/helpers/integrations_helper_spec.rb

@@ -98,4 +98,19 @@ RSpec.describe IntegrationsHelper do
       end
     end
   end
+
+  describe '#jira_issue_breadcrumb_link' do
+    let(:issue_reference) { nil }
+
+    subject { helper.jira_issue_breadcrumb_link(issue_reference) }
+
+    context 'when issue_reference contains HTML' do
+      let(:issue_reference) { "<script>alert('XSS')</script>" }
+
+      it 'escapes issue reference' do
+        is_expected.not_to include(issue_reference)
+        is_expected.to include(html_escape(issue_reference))
+      end
+    end
+  end
 end

spec/lib/banzai/filter/references/design_reference_filter_spec.rb

@@ -90,11 +90,8 @@ RSpec.describe Banzai::Filter::References::DesignReferenceFilter do
       [
         ['simple.png'],
         ['SIMPLE.PNG'],
-        ['has spaces.png'],
         ['has-hyphen.jpg'],
-        ['snake_case.svg'],
-        ['has "quotes".svg'],
-        ['has <special> characters [o].svg']
+        ['snake_case.svg']
       ]
     end
 
@@ -138,40 +135,25 @@ RSpec.describe Banzai::Filter::References::DesignReferenceFilter do
     end
   end
 
-  context 'a design with a quoted filename' do
-    let(:filename) { %q{A "very" good file.png} }
-    let(:design) { create(:design, :with_versions, issue: issue, filename: filename) }
-
-    it 'links to the design' do
-      expect(doc.css('a').first.attr('href'))
-        .to eq url_for_design(design)
-    end
-  end
-
   context 'internal reference' do
     it_behaves_like 'a reference containing an element node'
 
-    context 'the reference is valid' do
-      it_behaves_like 'a good link reference'
+    it_behaves_like 'a good link reference'
 
-      context 'the filename needs to be escaped' do
-        where(:filename) do
-          [
-            ['with some spaces.png'],
-            ['with <script>console.log("pwded")<%2Fscript>.png']
-          ]
-        end
+    context 'the filename contains invalid characters' do
+      where(:filename) do
+        [
+          ['with some spaces.png'],
+          ['with <script>console.log("pwded")<%2Fscript>.png'],
+          ['foo"bar.png'],
+          ['A "very" good file.png']
+        ]
+      end
 
-        with_them do
-          let(:design) { create(:design, :with_versions, filename: filename, issue: issue) }
-          let(:link) { doc.css('a').first }
+      with_them do
+        let(:design) { create(:design, :with_versions, filename: filename, issue: issue) }
 
-          it 'replaces the content with the reference, but keeps the link', :aggregate_failures do
-            expect(doc.text).to eq(CGI.unescapeHTML("Added #{design.to_reference}"))
-            expect(link.attr('title')).to eq(design.filename)
-            expect(link.attr('href')).to eq(design_url)
-          end
-        end
+        it_behaves_like 'a no-op filter'
       end
     end
 

spec/lib/gitlab/import_export/base/relation_factory_spec.rb

@@ -102,7 +102,7 @@ RSpec.describe Gitlab::ImportExport::Base::RelationFactory do
             "updated_at" => "2016-11-18T09:29:42.634Z",
             "user" => {
               "id" => 999,
-              "email" => new_user.email,
+              "public_email" => new_user.email,
               "username" => new_user.username
             }
           }

spec/lib/gitlab/import_export/group/legacy_tree_saver_spec.rb

@@ -83,11 +83,12 @@ RSpec.describe Gitlab::ImportExport::Group::LegacyTreeSaver do
         let(:user2) { create(:user, email: 'group@member.com') }
         let(:member_emails) do
           saved_group_json['members'].map do |pm|
-            pm['user']['email']
+            pm['user']['public_email']
           end
         end
 
         before do
+          user2.update(public_email: user2.email)
           group.add_developer(user2)
         end
 

spec/lib/gitlab/import_export/members_mapper_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do
            "user" =>
              {
                "id" => exported_user_id,
-               "email" => user2.email,
+               "public_email" => user2.email,
                "username" => 'test'
              },
            "user_id" => 19
@@ -122,7 +122,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do
                 "user" =>
                   {
                     "id" => exported_user_id,
-                    "email" => user2.email,
+                    "public_email" => user2.email,
                     "username" => 'test'
                   },
                 "user_id" => 19
@@ -157,6 +157,37 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do
           expect(members_mapper.map[exported_user_id]).to eq(user2.id)
         end
       end
+
+      context 'when user has email exported' do
+        let(:exported_members) do
+          [
+            {
+              "id" => 2,
+              "access_level" => 40,
+              "source_id" => 14,
+              "source_type" => source_type,
+              "notification_level" => 3,
+              "created_at" => "2016-03-11T10:21:44.822Z",
+              "updated_at" => "2016-03-11T10:21:44.822Z",
+              "created_by_id" => nil,
+              "invite_email" => nil,
+              "invite_token" => nil,
+              "invite_accepted_at" => nil,
+              "user" =>
+                {
+                  "id" => exported_user_id,
+                  "email" => user2.email,
+                  "username" => 'test'
+                },
+              "user_id" => 19
+            }
+          ]
+        end
+
+        it 'maps a member' do
+          expect(members_mapper.map[exported_user_id]).to eq(user2.id)
+        end
+      end
     end
 
     context 'when importable is Project' do
@@ -212,7 +243,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do
 
           before do
             group.add_users([user, user2], GroupMember::DEVELOPER)
-            user.update(email: 'invite@test.com')
+            user.update(public_email: 'invite@test.com')
           end
 
           it 'maps the importer' do

spec/lib/gitlab/import_export/project/relation_factory_spec.rb

@@ -119,7 +119,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_
         "updated_at" => "2016-11-18T09:29:42.634Z",
         "user" => {
           "id" => admin.id,
-          "email" => admin.email,
+          "public_email" => admin.email,
           "username" => admin.username
         }
       }
@@ -187,7 +187,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_
         "updated_at" => "2016-11-18T09:29:42.634Z",
         "user" => {
           "id" => admin.id,
-          "email" => admin.email,
+          "public_email" => admin.email,
           "username" => admin.username
         }
       }

spec/lib/gitlab/import_export/project/tree_saver_spec.rb

@@ -299,12 +299,13 @@ RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
 
         let(:member_emails) do
           emails = subject.map do |pm|
-            pm['user']['email']
+            pm['user']['public_email']
           end
           emails
         end
 
         before do
+          user2.update(public_email: user2.email)
           group.add_developer(user2)
         end
 

spec/migrations/20210811122206_update_external_project_bots_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!('update_external_project_bots')
+
+RSpec.describe UpdateExternalProjectBots, :migration do
+  def create_user(**extra_options)
+    defaults = { projects_limit: 0, email: "#{extra_options[:username]}@example.com" }
+
+    table(:users).create!(defaults.merge(extra_options))
+  end
+
+  it 'sets bot users as external if were created by external users' do
+    internal_user = create_user(username: 'foo')
+    external_user = create_user(username: 'bar', external: true)
+
+    internal_project_bot = create_user(username: 'foo2', user_type: 6, created_by_id: internal_user.id, external: false)
+    external_project_bot = create_user(username: 'bar2', user_type: 6, created_by_id: external_user.id, external: false)
+
+    migrate!
+
+    expect(table(:users).find(internal_project_bot.id).external).to eq false
+    expect(table(:users).find(external_project_bot.id).external).to eq true
+  end
+end

spec/models/design_management/design_spec.rb

@@ -572,6 +572,12 @@ RSpec.describe DesignManagement::Design do
       expect(described_class.link_reference_pattern).not_to match(url_for_designs(issue))
     end
 
+    it 'intentionally ignores filenames with any special character' do
+      design = build(:design, issue: issue, filename: '"invalid')
+
+      expect(described_class.link_reference_pattern).not_to match(url_for_design(design))
+    end
+
     where(:ext) do
       (described_class::SAFE_IMAGE_EXT + described_class::DANGEROUS_IMAGE_EXT).flat_map do |ext|
         [[ext], [ext.upcase]]
@@ -593,14 +599,6 @@ RSpec.describe DesignManagement::Design do
         )
       end
 
-      context 'the file needs to be encoded' do
-        let(:filename) { "my file.#{ext}" }
-
-        it 'extracts the encoded filename' do
-          expect(captures).to include('url_filename' => 'my%20file.' + ext)
-        end
-      end
-
       context 'the file is all upper case' do
         let(:filename) { "file.#{ext}".upcase }
 

spec/models/integrations/datadog_spec.rb

@@ -127,18 +127,6 @@ RSpec.describe Integrations::Datadog do
     end
   end
 
-  describe '#api_keys_url' do
-    subject { instance.api_keys_url }
-
-    it { is_expected.to eq("https://app.#{dd_site}/account/settings#api") }
-
-    context 'with unset datadog_site' do
-      let(:dd_site) { '' }
-
-      it { is_expected.to eq("https://docs.datadoghq.com/account_management/api-app-keys/") }
-    end
-  end
-
   describe '#test' do
     subject(:result) { saved_instance.test(pipeline_data) }
 

spec/requests/api/graphql/mutations/design_management/upload_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe "uploading designs" do
   let(:project) { issue.project }
   let(:files) { [fixture_file_upload("spec/fixtures/dk.png")] }
   let(:variables) { {} }
+  let(:mutation_response) { graphql_mutation_response(:design_management_upload) }
 
   def mutation
     input = {
@@ -21,14 +22,32 @@ RSpec.describe "uploading designs" do
     graphql_mutation(:design_management_upload, input)
   end
 
-  let(:mutation_response) { graphql_mutation_response(:design_management_upload) }
-
   before do
     enable_design_management
 
     project.add_developer(current_user)
   end
 
+  context 'when the input does not include a null value for each mapped file' do
+    let(:operations) { { query: mutation.query, variables: mutation.variables.merge(files: []) } }
+    let(:mapping) { { '1' => ['variables.files.0'] } }
+    let(:params) do
+      { '1' => files.first, operations: operations.to_json, map: mapping.to_json }
+    end
+
+    it 'returns an error' do
+      workhorse_post_with_file(api('/', current_user, version: 'graphql'),
+                               params: params,
+                               file_key: '1'
+                              )
+
+      expect(response).to have_attributes(
+        code: eq('400'),
+        body: include('out-of-bounds')
+      )
+    end
+  end
+
   it "returns an error if the user is not allowed to upload designs" do
     post_graphql_mutation_with_uploads(mutation, current_user: create(:user))
 

spec/requests/users_controller_spec.rb

@@ -274,7 +274,11 @@ RSpec.describe UsersController do
             stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
           end
 
-          it_behaves_like 'renders all public keys'
+          it 'redirects to sign in' do
+            get "/#{user.username}.keys"
+
+            expect(response).to redirect_to(new_user_session_path)
+          end
         end
       end
     end

spec/services/groups/destroy_service_spec.rb

@@ -152,56 +152,125 @@ RSpec.describe Groups::DestroyService do
     end
 
     context 'for shared groups within different hierarchies' do
-      let(:shared_with_group) { group }
-      let!(:shared_group) { create(:group, :private) }
-      let!(:shared_group_child) { create(:group, :private, parent: shared_group) }
-      let!(:shared_group_user) { create(:user) }
+      let(:group1) { create(:group, :private) }
+      let(:group2) { create(:group, :private) }
 
-      let!(:project) { create(:project, group: shared_group) }
-      let!(:project_child) { create(:project, group: shared_group_child) }
+      let(:group1_user) { create(:user) }
+      let(:group2_user) { create(:user) }
 
       before do
-        shared_group.add_user(shared_group_user, Gitlab::Access::OWNER)
-
-        create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group)
-        shared_with_group.refresh_members_authorized_projects
+        group1.add_user(group1_user, Gitlab::Access::OWNER)
+        group2.add_user(group2_user, Gitlab::Access::OWNER)
       end
 
-      context 'the shared group is deleted' do
-        it 'updates project authorization' do
-          expect(shared_group_user.can?(:read_project, project)).to eq(true)
-          expect(shared_group_user.can?(:read_project, project_child)).to eq(true)
+      context 'when a project is shared with a group' do
+        let!(:group1_project) { create(:project, :private, group: group1) }
+
+        before do
+          create(:project_group_link, project: group1_project, group: group2)
+        end
+
+        context 'and the shared group is deleted' do
+          it 'updates project authorizations so group2 users no longer have access', :aggregate_failures do
+            expect(group1_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group1_project)).to eq(true)
 
-          destroy_group(shared_group, shared_group_user, false)
+            destroy_group(group2, group2_user, false)
 
-          expect(shared_group_user.can?(:read_project, project)).to eq(false)
-          expect(shared_group_user.can?(:read_project, project_child)).to eq(false)
+            expect(group1_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group1_project)).to eq(false)
+          end
+
+          it 'calls the service to update project authorizations only with necessary user ids' do
+            expect(UserProjectAccessChangedService)
+              .to receive(:new).with(array_including(group2_user.id)).and_call_original
+
+            destroy_group(group2, group2_user, false)
+          end
         end
 
-        it 'does not make use of specific service to update project_authorizations records' do
-          expect(UserProjectAccessChangedService)
-            .not_to receive(:new).with(shared_group.user_ids_for_project_authorizations).and_call_original
+        context 'and the group is shared with another group' do
+          let(:group3) { create(:group, :private) }
+          let(:group3_user) { create(:user) }
+
+          before do
+            group3.add_user(group3_user, Gitlab::Access::OWNER)
+
+            create(:group_group_link, shared_group: group2, shared_with_group: group3)
+            group3.refresh_members_authorized_projects
+          end
+
+          it 'updates project authorizations so group2 and group3 users no longer have access', :aggregate_failures do
+            expect(group1_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group3_user.can?(:read_project, group1_project)).to eq(true)
+
+            destroy_group(group2, group2_user, false)
+
+            expect(group1_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group1_project)).to eq(false)
+            expect(group3_user.can?(:read_project, group1_project)).to eq(false)
+          end
 
-          destroy_group(shared_group, shared_group_user, false)
+          it 'calls the service to update project authorizations only with necessary user ids' do
+            expect(UserProjectAccessChangedService)
+              .to receive(:new).with(array_including(group2_user.id, group3_user.id)).and_call_original
+
+            destroy_group(group2, group2_user, false)
+          end
         end
       end
 
-      context 'the shared_with group is deleted' do
-        it 'updates project authorization' do
-          expect(user.can?(:read_project, project)).to eq(true)
-          expect(user.can?(:read_project, project_child)).to eq(true)
+      context 'when a group is shared with a group' do
+        let!(:group2_project) { create(:project, :private, group: group2) }
 
-          destroy_group(shared_with_group, user, false)
+        before do
+          create(:group_group_link, shared_group: group2, shared_with_group: group1)
+          group1.refresh_members_authorized_projects
+        end
+
+        context 'and the shared group is deleted' do
+          it 'updates project authorizations since the project has been deleted with the group', :aggregate_failures do
+            expect(group1_user.can?(:read_project, group2_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group2_project)).to eq(true)
+
+            destroy_group(group2, group2_user, false)
 
-          expect(user.can?(:read_project, project)).to eq(false)
-          expect(user.can?(:read_project, project_child)).to eq(false)
+            expect(group1_user.can?(:read_project, group2_project)).to eq(false)
+            expect(group2_user.can?(:read_project, group2_project)).to eq(false)
+          end
+
+          it 'does not call the service to update project authorizations' do
+            expect(UserProjectAccessChangedService).not_to receive(:new)
+
+            destroy_group(group2, group2_user, false)
+          end
         end
 
-        it 'makes use of a specific service to update project_authorizations records' do
-          expect(UserProjectAccessChangedService)
-            .to receive(:new).with(shared_with_group.user_ids_for_project_authorizations).and_call_original
+        context 'the shared_with group is deleted' do
+          let!(:group2_subgroup) { create(:group, :private, parent: group2)}
+          let!(:group2_subgroup_project) { create(:project, :private, group: group2_subgroup) }
 
-          destroy_group(shared_with_group, user, false)
+          it 'updates project authorizations so users of both groups lose access', :aggregate_failures do
+            expect(group1_user.can?(:read_project, group2_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group2_project)).to eq(true)
+            expect(group1_user.can?(:read_project, group2_subgroup_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group2_subgroup_project)).to eq(true)
+
+            destroy_group(group1, group1_user, false)
+
+            expect(group1_user.can?(:read_project, group2_project)).to eq(false)
+            expect(group2_user.can?(:read_project, group2_project)).to eq(true)
+            expect(group1_user.can?(:read_project, group2_subgroup_project)).to eq(false)
+            expect(group2_user.can?(:read_project, group2_subgroup_project)).to eq(true)
+          end
+
+          it 'calls the service to update project authorizations only with necessary user ids' do
+            expect(UserProjectAccessChangedService)
+              .to receive(:new).with([group1_user.id]).and_call_original
+
+            destroy_group(group1, group1_user, false)
+          end
         end
       end
     end

spec/services/jira_connect_subscriptions/create_service_spec.rb

@@ -7,8 +7,10 @@ RSpec.describe JiraConnectSubscriptions::CreateService do
   let(:current_user) { create(:user) }
   let(:group) { create(:group) }
   let(:path) { group.full_path }
+  let(:params) { { namespace_path: path, jira_user: jira_user } }
+  let(:jira_user) { double(:JiraUser, site_admin?: true) }
 
-  subject { described_class.new(installation, current_user, namespace_path: path).execute }
+  subject { described_class.new(installation, current_user, params).execute }
 
   before do
     group.add_maintainer(current_user)
@@ -24,6 +26,30 @@ RSpec.describe JiraConnectSubscriptions::CreateService do
     end
   end
 
+  context 'remote user does not have access' do
+    let(:jira_user) { double(site_admin?: false) }
+
+    it 'does not create a subscription' do
+      expect { subject }.not_to change { installation.subscriptions.count }
+    end
+
+    it 'returns error' do
+      expect(subject[:status]).to eq(:error)
+    end
+  end
+
+  context 'remote user cannot be retrieved' do
+    let(:jira_user) { nil }
+
+    it 'does not create a subscription' do
+      expect { subject }.not_to change { installation.subscriptions.count }
+    end
+
+    it 'returns error' do
+      expect(subject[:status]).to eq(:error)
+    end
+  end
+
   context 'when user does have access' do
     it 'creates a subscription' do
       expect { subject }.to change { installation.subscriptions.count }.from(0).to(1)

spec/services/resource_access_tokens/create_service_spec.rb

@@ -110,6 +110,18 @@ RSpec.describe ResourceAccessTokens::CreateService do
             expect(resource.members.developers.map(&:user_id)).to include(bot_user.id)
           end
         end
+
+        context 'when user is external' do
+          let(:user) { create(:user, :external) }
+
+          before do
+            project.add_maintainer(user)
+          end
+
+          it 'creates resource bot user with external status' do
+            expect(subject.payload[:access_token].user.external).to eq true
+          end
+        end
       end
 
       context 'personal access token' do

spec/support/shared_examples/lib/gitlab/import_export/relation_factory_shared_examples.rb

@@ -19,7 +19,7 @@ RSpec.shared_examples 'Notes user references' do
       'updated_at' => '2016-11-18T09:29:42.634Z',
       'user' => {
         'id' => 999,
-        'email' => mapped_user.email,
+        'public_email' => mapped_user.email,
         'username' => mapped_user.username
       }
     }