diff --git a/CHANGELOG.md b/CHANGELOG.md index aeb9d376f8..e8cfa50add 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.11.4 (2021-05-14) + +### Fixed (3 changes) + +- Fix N+1 SQL queries in PipelinesController#show. !60794 +- Omit trailing slash when proxying pre-authorized routes with no suffix. !61638 +- Omit trailing slash when checking allowed requests in the read-only middleware. !61641 + + ## 13.11.3 (2021-04-30) ### Fixed (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index fd9f285315..33498dc40c 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.11.3 \ No newline at end of file +13.11.5 \ No newline at end of file diff --git a/Gemfile.lock b/Gemfile.lock index 007ca87b46..059dcbed1d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -144,7 +144,7 @@ GEM coderay (>= 1.0.0) erubi (>= 1.0.0) rack (>= 0.9.0) - bindata (2.4.8) + bindata (2.4.10) binding_ninja (0.2.3) bootsnap (1.4.6) msgpack (~> 1.0) diff --git a/VERSION b/VERSION index fd9f285315..33498dc40c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -13.11.3 \ No newline at end of file +13.11.5 \ No newline at end of file diff --git a/app/assets/javascripts/notebook/cells/markdown.vue b/app/assets/javascripts/notebook/cells/markdown.vue index c09db6851e..97b0557cbe 100644 --- a/app/assets/javascripts/notebook/cells/markdown.vue +++ b/app/assets/javascripts/notebook/cells/markdown.vue @@ -160,6 +160,7 @@ export default { 'var', ], ALLOWED_ATTR: ['class', 'style', 'href', 'src'], + ALLOW_DATA_ATTR: false, }); }, }, diff --git a/app/controllers/jira_connect/app_descriptor_controller.rb b/app/controllers/jira_connect/app_descriptor_controller.rb index 137f830e40..fee8b43aa6 100644 --- a/app/controllers/jira_connect/app_descriptor_controller.rb +++ b/app/controllers/jira_connect/app_descriptor_controller.rb @@ -31,6 +31,7 @@ class JiraConnect::AppDescriptorController < JiraConnect::ApplicationController scopes: %w(READ WRITE DELETE), apiVersion: 1, apiMigrations: { + 'context-qsh': true, gdpr: true } } diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index 857f36e383..ddf70c1892 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -14,8 +14,9 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController if pre_auth.authorizable? if skip_authorization? || matching_token? auth = authorization.authorize + parsed_redirect_uri = URI.parse(auth.redirect_uri) session.delete(:user_return_to) - redirect_to auth.redirect_uri + render "doorkeeper/authorizations/redirect", locals: { redirect_uri: parsed_redirect_uri }, layout: false else render "doorkeeper/authorizations/new" end diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index ee1e10221e..9f326ef59f 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -216,7 +216,7 @@ class Projects::PipelinesController < Projects::ApplicationController end def render_show - @stages = @pipeline.stages.with_latest_and_retried_statuses + @stages = @pipeline.stages respond_to do |format| format.html do diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index ad206d0e5b..6c19fcc912 100644 --- a/app/helpers/markup_helper.rb +++ b/app/helpers/markup_helper.rb @@ -118,6 +118,7 @@ module MarkupHelper def markup(file_name, text, context = {}) context[:project] ||= @project + context[:text_source] ||= :blob html = context.delete(:rendered) || markup_unsafe(file_name, text, context) prepare_for_rendering(html, context) end diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index 9dd75150ac..5ae97dcd49 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -6,6 +6,7 @@ module Ci include Importable include Ci::HasStatus include Gitlab::OptimisticLocking + include Presentable enum status: Ci::HasStatus::STATUSES_ENUM @@ -22,12 +23,6 @@ module Ci scope :ordered, -> { order(position: :asc) } scope :in_pipelines, ->(pipelines) { where(pipeline: pipelines) } scope :by_name, ->(names) { where(name: names) } - scope :with_latest_and_retried_statuses, -> do - includes( - latest_statuses: [:pipeline, project: :namespace], - retried_statuses: [:pipeline, project: :namespace] - ) - end with_options unless: :importing? do validates :project, presence: true diff --git a/app/policies/concerns/policy_actor.rb b/app/policies/concerns/policy_actor.rb index 75849fb10c..d254b87ae1 100644 --- a/app/policies/concerns/policy_actor.rb +++ b/app/policies/concerns/policy_actor.rb @@ -80,6 +80,10 @@ module PolicyActor def can_read_all_resources? false end + + def password_expired? + false + end end PolicyActor.prepend_if_ee('EE::PolicyActor') diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index d16c4734b2..2bdccf8304 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -15,6 +15,10 @@ class GlobalPolicy < BasePolicy @user&.required_terms_not_accepted? end + condition(:password_expired, scope: :user) do + @user&.password_expired? + end + condition(:project_bot, scope: :user) { @user&.project_bot? } condition(:migration_bot, scope: :user) { @user&.migration_bot? } @@ -73,6 +77,12 @@ class GlobalPolicy < BasePolicy prevent :access_git end + rule { password_expired }.policy do + prevent :access_api + prevent :access_git + prevent :use_slash_commands + end + rule { can_create_group }.policy do enable :create_group end diff --git a/app/presenters/ci/stage_presenter.rb b/app/presenters/ci/stage_presenter.rb new file mode 100644 index 0000000000..9ec3f8d153 --- /dev/null +++ b/app/presenters/ci/stage_presenter.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Ci + class StagePresenter < Gitlab::View::Presenter::Delegated + presents :stage + + def latest_ordered_statuses + preload_statuses(stage.statuses.latest_ordered) + end + + def retried_ordered_statuses + preload_statuses(stage.statuses.retried_ordered) + end + + private + + def preload_statuses(statuses) + loaded_statuses = statuses.load + statuses.tap do |statuses| + # rubocop: disable CodeReuse/ActiveRecord + ActiveRecord::Associations::Preloader.new.preload(preloadable_statuses(loaded_statuses), %w[pipeline tags job_artifacts_archive metadata]) + # rubocop: enable CodeReuse/ActiveRecord + end + end + + def preloadable_statuses(statuses) + statuses.reject do |status| + status.instance_of?(::GenericCommitStatus) || status.instance_of?(::Ci::Bridge) + end + end + end +end diff --git a/app/serializers/member_entity.rb b/app/serializers/member_entity.rb index 6cbdaeea5e..3b75e876b9 100644 --- a/app/serializers/member_entity.rb +++ b/app/serializers/member_entity.rb @@ -40,7 +40,9 @@ class MemberEntity < Grape::Entity expose :valid_level_roles, as: :valid_roles - expose :user, if: -> (member) { member.user.present? }, using: MemberUserEntity + expose :user, if: -> (member) { member.user.present? } do |member, options| + MemberUserEntity.represent(member.user, source: options[:source]) + end expose :invite, if: -> (member) { member.invite? } do expose :email do |member| diff --git a/app/views/doorkeeper/authorizations/redirect.html.haml b/app/views/doorkeeper/authorizations/redirect.html.haml new file mode 100644 index 0000000000..9580f33c88 --- /dev/null +++ b/app/views/doorkeeper/authorizations/redirect.html.haml @@ -0,0 +1,8 @@ +%h3.page-title= _("Redirecting") + +%div + %a{ :href => redirect_uri } Click here to redirect to #{redirect_uri} + += javascript_tag do + :plain + window.location= "#{redirect_uri}"; diff --git a/app/views/projects/stage/_stage.html.haml b/app/views/projects/stage/_stage.html.haml index 92bfd5a48a..387c8fb323 100644 --- a/app/views/projects/stage/_stage.html.haml +++ b/app/views/projects/stage/_stage.html.haml @@ -1,3 +1,5 @@ +- stage = stage.present(current_user: current_user) + %tr %th{ colspan: 10 } %strong @@ -6,8 +8,8 @@ = ci_icon_for_status(stage.status)   = stage.name.titleize -= render stage.latest_statuses, stage: false, ref: false, pipeline_link: false, allow_retry: true -= render stage.retried_statuses, stage: false, ref: false, pipeline_link: false, retried: true += render stage.latest_ordered_statuses, stage: false, ref: false, pipeline_link: false, allow_retry: true += render stage.retried_ordered_statuses, stage: false, ref: false, pipeline_link: false, retried: true %tr %td{ colspan: 10 }   diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb index e8479bc6aa..61e357808d 100644 --- a/config/initializers/lograge.rb +++ b/config/initializers/lograge.rb @@ -18,6 +18,7 @@ unless Gitlab::Runtime.sidekiq? data[:db_duration_s] = Gitlab::Utils.ms_to_round_sec(data.delete(:db)) if data[:db] data[:view_duration_s] = Gitlab::Utils.ms_to_round_sec(data.delete(:view)) if data[:view] data[:duration_s] = Gitlab::Utils.ms_to_round_sec(data.delete(:duration)) if data[:duration] + data[:location] = Gitlab::Utils.removes_sensitive_data_from_url(data[:location]) if data[:location] # Remove empty hashes to prevent type mismatches # These are set to empty hashes in Lograge's ActionCable subscriber diff --git a/db/migrate/20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb b/db/migrate/20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb new file mode 100644 index 0000000000..2da84301a7 --- /dev/null +++ b/db/migrate/20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class ScheduleUpdateUsersWhereTwoFactorAuthRequiredFromGroup < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + MIGRATION = 'UpdateUsersWhereTwoFactorAuthRequiredFromGroup' + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 10_000 + INDEX_NAME = 'index_users_require_two_factor_authentication_from_group_false' + + disable_ddl_transaction! + + class User < ActiveRecord::Base + include EachBatch + + self.table_name = 'users' + end + + def up + add_concurrent_index :users, + :require_two_factor_authentication_from_group, + where: 'require_two_factor_authentication_from_group = FALSE', + name: INDEX_NAME + + relation = User.where(require_two_factor_authentication_from_group: false) + + queue_background_migration_jobs_by_range_at_intervals( + relation, MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE) + end + + def down + remove_concurrent_index_by_name :users, INDEX_NAME + end +end diff --git a/db/schema_migrations/20210519154058 b/db/schema_migrations/20210519154058 new file mode 100644 index 0000000000..9bd277e92d --- /dev/null +++ b/db/schema_migrations/20210519154058 @@ -0,0 +1 @@ +bdd82fc5cb2bbb322125c153c741002725853e23cd0ae0edbfd80563a4a87f2f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index db46364581..e09851aa10 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24170,6 +24170,8 @@ CREATE INDEX index_users_ops_dashboard_projects_on_project_id ON users_ops_dashb CREATE UNIQUE INDEX index_users_ops_dashboard_projects_on_user_id_and_project_id ON users_ops_dashboard_projects USING btree (user_id, project_id); +CREATE INDEX index_users_require_two_factor_authentication_from_group_false ON users USING btree (require_two_factor_authentication_from_group) WHERE (require_two_factor_authentication_from_group = false); + CREATE INDEX index_users_security_dashboard_projects_on_user_id ON users_security_dashboard_projects USING btree (user_id); CREATE INDEX index_users_star_projects_on_project_id ON users_star_projects USING btree (project_id); diff --git a/doc/user/profile/img/notification_global_settings.png b/doc/user/profile/img/notification_global_settings.png deleted file mode 100644 index 699f726c44..0000000000 Binary files a/doc/user/profile/img/notification_global_settings.png and /dev/null differ diff --git a/doc/user/profile/img/notification_global_settings_v13_12.png b/doc/user/profile/img/notification_global_settings_v13_12.png new file mode 100644 index 0000000000..2989543c2d Binary files /dev/null and b/doc/user/profile/img/notification_global_settings_v13_12.png differ diff --git a/doc/user/profile/notifications.md b/doc/user/profile/notifications.md index 4d890e249e..c0d232ba49 100644 --- a/doc/user/profile/notifications.md +++ b/doc/user/profile/notifications.md @@ -39,31 +39,32 @@ You can tune the notifications you receive by combining your notification settin - [Notification scope](#notification-scope) - [Notification levels](#notification-levels) -### Editing notification settings +## Editing notification settings To edit your notification settings: -1. Click on your profile picture and select **Settings**. -1. Click **Notifications** in the left sidebar. +1. In the top-right corner, select your avatar. +1. Select **Preferences**. +1. In the left sidebar, select **Notifications**. 1. Edit the desired notification settings. Edited settings are automatically saved and enabled. These notification settings apply only to you. They do not affect the notifications received by anyone else in the same project or group. -![notification settings](img/notification_global_settings.png) - ## Global notification settings Your **Global notification settings** are the default settings unless you select different values for a project or a group. -- **Notification email**: The email address your notifications are sent to. -- **Global notification level**: The default [notification level](#notification-levels) +- **Notification email**: the email address your notifications are sent to. +- **Receive product marketing emails**: select this check box to receive + [periodic emails](#product-marketing-emails) about GitLab features. +- **Global notification level**: the default [notification level](#notification-levels) which applies to all your notifications. -- **Receive product marketing emails**: Select this check box to receive periodic - emails about GitLab features. -- **Receive notifications about your own activity**: Select this check box to receive +- **Receive notifications about your own activity**: select this check box to receive notifications about your own activity. Not selected by default. +![notification settings](img/notification_global_settings_v13_12.png) + ### Notification scope You can tune the scope of your notifications by selecting different notification levels for each project and group. @@ -85,15 +86,16 @@ You can select a notification level for each project to help you closely monitor To select a notification level for a project, use either of these methods: -1. Click on your profile picture and select **Settings**. -1. Click **Notifications** in the left sidebar. +1. In the top-right corner, select your avatar. +1. Select **Preferences**. +1. In the left sidebar, select **Notifications**. 1. Locate the project in the **Projects** section. 1. Select the desired [notification level](#notification-levels). Or: -1. Navigate to the project's page. -1. Click the notification dropdown, marked with a bell icon. +1. Go to your project. +1. Select the notification dropdown, marked with a bell icon (**{notifications}**). 1. Select the desired [notification level](#notification-levels). @@ -109,15 +111,16 @@ You can select a notification level and email address for each group. To select a notification level for a group, use either of these methods: -1. Click on your profile picture and select **Settings**. -1. Click **Notifications** in the left sidebar. +1. In the top-right corner, select your avatar. +1. Select **Preferences**. +1. In the left sidebar, select **Notifications**. 1. Locate the project in the **Groups** section. 1. Select the desired [notification level](#notification-levels). --- -1. Navigate to the group's page. -1. Click the notification dropdown, marked with a bell icon. +1. Go to your group. +1. Select the notification dropdown, marked with a bell icon (**{notifications}**). 1. Select the desired [notification level](#notification-levels). ##### Group notification email address @@ -126,8 +129,9 @@ To select a notification level for a group, use either of these methods: You can select an email address to receive notifications for each group you belong to. This could be useful, for example, if you work freelance, and want to keep email about clients' projects separate. -1. Click on your profile picture and select **Settings**. -1. Click **Notifications** in the left sidebar. +1. In the top-right corner, select your avatar. +1. Select **Preferences**. +1. In the left sidebar, select **Notifications**. 1. Locate the project in the **Groups** section. 1. Select the desired email address. @@ -144,6 +148,27 @@ For each project and group you can select one of the following levels: | Disabled | Turns off notifications. | | Custom | Receive notifications for custom selected events. | +### Product marketing emails + +You can receive emails that teach you about various GitLab features. +This is enabled by default. + +To opt out, [edit your notification settings](#editing-notification-settings) and clear the +**Receive product marketing emails** checkbox. + +Disabling these emails does not disable all emails. +Learn how to [opt out of all emails from GitLab](#opt-out-of-all-gitlab-emails). + +#### Self-managed product marketing emails **(FREE SELF)** + +The self-managed installation generates and automatically sends these emails based on user actions. +Turning this on does not cause your GitLab instance or your company to send any personal information to +GitLab Inc. + +An instance administrator can configure this setting for all users. If you choose to opt out, your +setting overrides the instance-wide setting, even when an administrator later enables these emails +for all users. + ## Notification events Users are notified of the following events: @@ -176,7 +201,11 @@ To enable notifications on one specific issue, merge request or epic, you need t - **Disable**: If you are receiving notifications for updates to that issue but no longer want to receive them, unsubscribe from it. -Configuring this notification on an epic doesn't make you automatically subscribed to the issue that are linked to the epic. +Disabling this toggle only unsubscribes you from updates related to this issue, merge request, or epic. +Learn how to [opt out of all emails from GitLab](#opt-out-of-all-gitlab-emails). + +Enabling this notification on an epic doesn't automatically subscribe you to the issues linked +to the epic. For most events, the notification is sent to: @@ -229,7 +258,7 @@ If the title or description of an issue or merge request is changed, notifications are sent to any **new** mentions by `@username` as if they had been mentioned in the original text. -You don't receive notifications for Issues, Merge Requests or Milestones created +You don't receive notifications for issues, merge requests or milestones created by yourself (except when an issue is due). You only receive automatic notifications when somebody else comments or adds changes to the ones that you've created or mentions you. @@ -250,6 +279,21 @@ The participants are: - Authors of comments on the design. - Anyone that is `@mentioned` in a comment on the design. +## Opt out of all GitLab emails + +If you no longer wish to receive any email notifications: + +1. [Go to the Notifications settings page.](#editing-notification-settings) +1. Clear the **Receive product marketing emails** checkbox. +1. Set your **Global notification level** to **Disabled**. +1. Clear the **Receive notifications about your own activity** checkbox. +1. If you belong to any groups or projects, set their notification setting to **Global** or + **Disabled**. + +On self-managed installations, even after doing this, your instance administrator +[can still email you](../../tools/email.md). +To unsubscribe, select the unsubscribe link in one of these emails. + ## Filtering email Notification email messages include GitLab-specific headers. You can filter the notification emails based on the content of these headers to better manage your notifications. For example, you could filter all emails for a specific project where you are being assigned either a merge request or issue. diff --git a/doc/user/project/description_templates.md b/doc/user/project/description_templates.md index 3acef242ce..4a2bd56b7b 100644 --- a/doc/user/project/description_templates.md +++ b/doc/user/project/description_templates.md @@ -106,9 +106,10 @@ instance or the project's parent groups. ### Set instance-level description templates **(PREMIUM SELF)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52360) in GitLab 13.9. -> - It's [deployed behind a feature flag](../feature_flags.md), disabled by default. -> - [Became enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56737) in GitLab 13.11. -> - It's enabled by default on GitLab.com. +> - [Deployed behind a feature flag](../feature_flags.md), disabled by default. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56737) in GitLab 13.11. +> - Enabled by default on GitLab.com. +> - Recommended for production use. > - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-issue-and-merge-request-description-templates-at-group-and-instance-level). **(PREMIUM SELF)** You can set a description template at the **instance level** for issues @@ -131,9 +132,10 @@ Learn more about [instance template repository](../admin_area/settings/instance_ ### Set group-level description templates **(PREMIUM)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52360) in GitLab 13.9. -> - It's [deployed behind a feature flag](../feature_flags.md), disabled by default. -> - [Became enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56737) in GitLab 13.11. -> - It's enabled by default on GitLab.com. +> - [Deployed behind a feature flag](../feature_flags.md), disabled by default. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56737) in GitLab 13.11. +> - Enabled by default on GitLab.com. +> - Recommended for production use. > - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-issue-and-merge-request-description-templates-at-group-and-instance-level). **(PREMIUM SELF)** With **group-level** description templates, you can store your templates in a single repository and @@ -230,26 +232,26 @@ it's very hard to read otherwise.) /assign @qa-tester ``` -## Enable or disable issue and merge request description templates at group and instance level +## Enable or disable issue and merge request description templates at group and instance level **(PREMIUM SELF)** Setting issue and merge request description templates at group and instance levels -is under development and not ready for production use. It is deployed behind a +is under development but ready for production use. It is deployed behind a feature flag that is **enabled by default**. [GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) can disable it. -To enable it: - -```ruby -Feature.enable(:inherited_issuable_templates) -``` - To disable it: ```ruby Feature.disable(:inherited_issuable_templates) ``` +To enable it: + +```ruby +Feature.enable(:inherited_issuable_templates) +``` + The feature flag affects these features: - Setting a templates project as issue and merge request description templates source at group level. diff --git a/lib/api/lint.rb b/lib/api/lint.rb index e0806674c6..945cdf3edb 100644 --- a/lib/api/lint.rb +++ b/lib/api/lint.rb @@ -11,7 +11,7 @@ module API optional :include_merged_yaml, type: Boolean, desc: 'Whether or not to include merged CI config yaml in the response' end post '/lint' do - unauthorized! if Gitlab::CurrentSettings.signup_disabled? && current_user.nil? + unauthorized! if (Gitlab::CurrentSettings.signup_disabled? || Gitlab::CurrentSettings.signup_limited?) && current_user.nil? result = Gitlab::Ci::YamlProcessor.new(params[:content], user: current_user).execute diff --git a/lib/banzai/filter/absolute_link_filter.rb b/lib/banzai/filter/absolute_link_filter.rb index a9bdb004c4..cc7bf3ed55 100644 --- a/lib/banzai/filter/absolute_link_filter.rb +++ b/lib/banzai/filter/absolute_link_filter.rb @@ -6,10 +6,13 @@ module Banzai module Filter # HTML filter that converts relative urls into absolute ones. class AbsoluteLinkFilter < HTML::Pipeline::Filter + CSS = 'a.gfm' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + def call return doc unless context[:only_path] == false - doc.search('a.gfm').each do |el| + doc.xpath(XPATH).each do |el| process_link_attr el.attribute('href') end diff --git a/lib/banzai/filter/ascii_doc_post_processing_filter.rb b/lib/banzai/filter/ascii_doc_post_processing_filter.rb index 09f0fd7df4..83c729e13b 100644 --- a/lib/banzai/filter/ascii_doc_post_processing_filter.rb +++ b/lib/banzai/filter/ascii_doc_post_processing_filter.rb @@ -3,14 +3,20 @@ module Banzai module Filter class AsciiDocPostProcessingFilter < HTML::Pipeline::Filter + CSS_MATH = '[data-math-style]' + XPATH_MATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_MATH).freeze + CSS_MERM = '[data-mermaid-style]' + XPATH_MERM = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_MERM).freeze + def call - doc.search('[data-math-style]').each do |node| + doc.xpath(XPATH_MATH).each do |node| node.set_attribute('class', 'code math js-render-math') end - doc.search('[data-mermaid-style]').each do |node| + doc.xpath(XPATH_MERM).each do |node| node.set_attribute('class', 'js-render-mermaid') end + doc end end diff --git a/lib/banzai/filter/base_relative_link_filter.rb b/lib/banzai/filter/base_relative_link_filter.rb index fd526df4c4..60d09b69a1 100644 --- a/lib/banzai/filter/base_relative_link_filter.rb +++ b/lib/banzai/filter/base_relative_link_filter.rb @@ -7,23 +7,20 @@ module Banzai class BaseRelativeLinkFilter < HTML::Pipeline::Filter include Gitlab::Utils::StrongMemoize + CSS = 'a:not(.gfm), img:not(.gfm), video:not(.gfm), audio:not(.gfm)' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + protected def linkable_attributes strong_memoize(:linkable_attributes) do attrs = [] - attrs += doc.search('a:not(.gfm)').map do |el| - el.attribute('href') + attrs += doc.xpath(XPATH).flat_map do |el| + [el.attribute('href'), el.attribute('src'), el.attribute('data-src')] end - attrs += doc.search('img:not(.gfm), video:not(.gfm), audio:not(.gfm)').flat_map do |el| - [el.attribute('src'), el.attribute('data-src')] - end - - attrs.reject do |attr| - attr.blank? || attr.value.start_with?('//') - end + attrs.reject { |attr| attr.blank? || attr.value.start_with?('//') } end end diff --git a/lib/banzai/filter/color_filter.rb b/lib/banzai/filter/color_filter.rb index 0aca744163..58e9b8cdba 100644 --- a/lib/banzai/filter/color_filter.rb +++ b/lib/banzai/filter/color_filter.rb @@ -7,8 +7,11 @@ module Banzai class ColorFilter < HTML::Pipeline::Filter COLOR_CHIP_CLASS = 'gfm-color_chip' + CSS = 'code' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + def call - doc.css('code').each do |node| + doc.xpath(XPATH).each do |node| color = ColorParser.parse(node.content) node << color_chip(color) if color end diff --git a/lib/banzai/filter/custom_emoji_filter.rb b/lib/banzai/filter/custom_emoji_filter.rb index 1ee8f4e31e..e26c5d36f2 100644 --- a/lib/banzai/filter/custom_emoji_filter.rb +++ b/lib/banzai/filter/custom_emoji_filter.rb @@ -9,7 +9,7 @@ module Banzai return doc unless context[:project] return doc unless Feature.enabled?(:custom_emoji, context[:project]) - doc.search(".//text()").each do |node| + doc.xpath('descendant-or-self::text()').each do |node| content = node.to_html next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS) diff --git a/lib/banzai/filter/emoji_filter.rb b/lib/banzai/filter/emoji_filter.rb index 8952a3ff6b..9d24bf028b 100644 --- a/lib/banzai/filter/emoji_filter.rb +++ b/lib/banzai/filter/emoji_filter.rb @@ -11,7 +11,7 @@ module Banzai IGNORE_UNICODE_EMOJIS = %w(™ © ®).freeze def call - doc.search(".//text()").each do |node| + doc.xpath('descendant-or-self::text()').each do |node| content = node.to_html next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS) diff --git a/lib/banzai/filter/footnote_filter.rb b/lib/banzai/filter/footnote_filter.rb index 5474242e03..0f856dc0eb 100644 --- a/lib/banzai/filter/footnote_filter.rb +++ b/lib/banzai/filter/footnote_filter.rb @@ -23,17 +23,23 @@ module Banzai FOOTNOTE_LINK_REFERENCE_PATTERN = /\A#{FOOTNOTE_LINK_ID_PREFIX}\d+\z/.freeze FOOTNOTE_START_NUMBER = 1 + CSS_SECTION = "ol > li[id=#{FOOTNOTE_ID_PREFIX}#{FOOTNOTE_START_NUMBER}]" + XPATH_SECTION = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_SECTION).freeze + CSS_FOOTNOTE = 'sup > a[id]' + XPATH_FOOTNOTE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_FOOTNOTE).freeze + def call - return doc unless first_footnote = doc.at_css("ol > li[id=#{fn_id(FOOTNOTE_START_NUMBER)}]") + return doc unless first_footnote = doc.at_xpath(XPATH_SECTION) # Sanitization stripped off the section wrapper - add it back in first_footnote.parent.wrap('
') rand_suffix = "-#{random_number}" modified_footnotes = {} - doc.css('sup > a[id]').each do |link_node| + doc.xpath(XPATH_FOOTNOTE).each do |link_node| ref_num = link_node[:id].delete_prefix(FOOTNOTE_LINK_ID_PREFIX) - footnote_node = doc.at_css("li[id=#{fn_id(ref_num)}]") + node_xpath = Gitlab::Utils::Nokogiri.css_to_xpath("li[id=#{fn_id(ref_num)}]") + footnote_node = doc.at_xpath(node_xpath) if INTEGER_PATTERN.match?(ref_num) && (footnote_node || modified_footnotes[ref_num]) link_node[:href] += rand_suffix diff --git a/lib/banzai/filter/gollum_tags_filter.rb b/lib/banzai/filter/gollum_tags_filter.rb index 6de9f2b86f..0548d5a999 100644 --- a/lib/banzai/filter/gollum_tags_filter.rb +++ b/lib/banzai/filter/gollum_tags_filter.rb @@ -60,7 +60,7 @@ module Banzai IGNORED_ANCESTOR_TAGS = %w(pre code tt).to_set def call - doc.search(".//text()").each do |node| + doc.xpath('descendant-or-self::text()').each do |node| next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS) next unless node.content =~ TAGS_PATTERN diff --git a/lib/banzai/filter/image_lazy_load_filter.rb b/lib/banzai/filter/image_lazy_load_filter.rb index d8b9eb29cf..916c135b77 100644 --- a/lib/banzai/filter/image_lazy_load_filter.rb +++ b/lib/banzai/filter/image_lazy_load_filter.rb @@ -6,8 +6,11 @@ module Banzai # HTML filter that moves the value of image `src` attributes to `data-src` # so they can be lazy loaded. class ImageLazyLoadFilter < HTML::Pipeline::Filter + CSS = 'img' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + def call - doc.xpath('descendant-or-self::img').each do |img| + doc.xpath(XPATH).each do |img| img.add_class('lazy') img['data-src'] = img['src'] img['src'] = LazyImageTagHelper.placeholder_image diff --git a/lib/banzai/filter/inline_diff_filter.rb b/lib/banzai/filter/inline_diff_filter.rb index 5a1c0bee32..e47ff15e7b 100644 --- a/lib/banzai/filter/inline_diff_filter.rb +++ b/lib/banzai/filter/inline_diff_filter.rb @@ -7,7 +7,7 @@ module Banzai IGNORED_ANCESTOR_TAGS = %w(pre code tt).to_set def call - doc.search(".//text()").each do |node| + doc.xpath('descendant-or-self::text()').each do |node| next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS) content = node.to_html diff --git a/lib/banzai/filter/inline_metrics_redactor_filter.rb b/lib/banzai/filter/inline_metrics_redactor_filter.rb index 2259115acf..b256815ae8 100644 --- a/lib/banzai/filter/inline_metrics_redactor_filter.rb +++ b/lib/banzai/filter/inline_metrics_redactor_filter.rb @@ -8,6 +8,7 @@ module Banzai include Gitlab::Utils::StrongMemoize METRICS_CSS_CLASS = '.js-render-metrics' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(METRICS_CSS_CLASS).freeze EMBED_LIMIT = 100 Route = Struct.new(:regex, :permission) @@ -41,7 +42,7 @@ module Banzai # @return [Nokogiri::XML::NodeSet] def nodes strong_memoize(:nodes) do - nodes = doc.css(METRICS_CSS_CLASS) + nodes = doc.xpath(XPATH) nodes.drop(EMBED_LIMIT).each(&:remove) nodes diff --git a/lib/banzai/filter/kroki_filter.rb b/lib/banzai/filter/kroki_filter.rb index dbd4de32a4..3803302c32 100644 --- a/lib/banzai/filter/kroki_filter.rb +++ b/lib/banzai/filter/kroki_filter.rb @@ -15,10 +15,11 @@ module Banzai .map { |diagram_type| %(pre[lang="#{diagram_type}"] > code) } .join(', ') - return doc unless doc.at(diagram_selectors) + xpath = Gitlab::Utils::Nokogiri.css_to_xpath(diagram_selectors) + return doc unless doc.at_xpath(xpath) diagram_format = "svg" - doc.css(diagram_selectors).each do |node| + doc.xpath(xpath).each do |node| diagram_type = node.parent['lang'] img_tag = Nokogiri::HTML::DocumentFragment.parse(%()) node.parent.replace(img_tag) diff --git a/lib/banzai/filter/markdown_post_escape_filter.rb b/lib/banzai/filter/markdown_post_escape_filter.rb index ad32e9afbf..b69afdcfeb 100644 --- a/lib/banzai/filter/markdown_post_escape_filter.rb +++ b/lib/banzai/filter/markdown_post_escape_filter.rb @@ -8,6 +8,11 @@ module Banzai NOT_LITERAL_REGEX = %r{#{LITERAL_KEYWORD}-((%5C|\\).+?)-#{LITERAL_KEYWORD}}.freeze SPAN_REGEX = %r{(.*?)}.freeze + CSS_A = 'a' + XPATH_A = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_A).freeze + CSS_CODE = 'code' + XPATH_CODE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_CODE).freeze + def call return doc unless result[:escaped_literals] @@ -24,12 +29,12 @@ module Banzai # Banzai::Renderer::CommonMark::HTML. However, we eventually want to use # the built-in compiled renderer, rather than the ruby version, for speed. # So let's do this work here. - doc.css('a').each do |node| + doc.xpath(XPATH_A).each do |node| node.attributes['href'].value = node.attributes['href'].value.gsub(SPAN_REGEX, '\1') if node.attributes['href'] node.attributes['title'].value = node.attributes['title'].value.gsub(SPAN_REGEX, '\1') if node.attributes['title'] end - doc.css('code').each do |node| + doc.xpath(XPATH_CODE).each do |node| node.attributes['lang'].value = node.attributes['lang'].value.gsub(SPAN_REGEX, '\1') if node.attributes['lang'] end diff --git a/lib/banzai/filter/math_filter.rb b/lib/banzai/filter/math_filter.rb index 2247984b86..53dafe45fb 100644 --- a/lib/banzai/filter/math_filter.rb +++ b/lib/banzai/filter/math_filter.rb @@ -10,6 +10,11 @@ module Banzai # HTML filter that adds class="code math" and removes the dollar sign in $`2+2`$. # class MathFilter < HTML::Pipeline::Filter + CSS_MATH = 'pre.code.language-math' + XPATH_MATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_MATH).freeze + CSS_CODE = 'code' + XPATH_CODE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_CODE).freeze + # Attribute indicating inline or display math. STYLE_ATTRIBUTE = 'data-math-style' @@ -21,7 +26,7 @@ module Banzai DOLLAR_SIGN = '$' def call - doc.css('code').each do |code| + doc.xpath(XPATH_CODE).each do |code| closing = code.next opening = code.previous @@ -39,7 +44,7 @@ module Banzai end end - doc.css('pre.code.language-math').each do |el| + doc.xpath(XPATH_MATH).each do |el| el[STYLE_ATTRIBUTE] = 'display' el[:class] += " #{TAG_CLASS}" end diff --git a/lib/banzai/filter/mermaid_filter.rb b/lib/banzai/filter/mermaid_filter.rb index f0adb83af8..aaaf851ccf 100644 --- a/lib/banzai/filter/mermaid_filter.rb +++ b/lib/banzai/filter/mermaid_filter.rb @@ -4,8 +4,11 @@ module Banzai module Filter class MermaidFilter < HTML::Pipeline::Filter + CSS = 'pre[lang="mermaid"] > code' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + def call - doc.css('pre[lang="mermaid"] > code').add_class('js-render-mermaid') + doc.xpath(XPATH).add_class('js-render-mermaid') doc end diff --git a/lib/banzai/filter/plantuml_filter.rb b/lib/banzai/filter/plantuml_filter.rb index 37d4126c1b..93370178a6 100644 --- a/lib/banzai/filter/plantuml_filter.rb +++ b/lib/banzai/filter/plantuml_filter.rb @@ -8,12 +8,15 @@ module Banzai # HTML that replaces all `code plantuml` tags with PlantUML img tags. # class PlantumlFilter < HTML::Pipeline::Filter + CSS = 'pre > code[lang="plantuml"]' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + def call - return doc unless settings.plantuml_enabled? && doc.at('pre > code[lang="plantuml"]') + return doc unless settings.plantuml_enabled? && doc.at_xpath(XPATH) plantuml_setup - doc.css('pre > code[lang="plantuml"]').each do |node| + doc.xpath(XPATH).each do |node| img_tag = Nokogiri::HTML::DocumentFragment.parse( Asciidoctor::PlantUml::Processor.plantuml_content(node.content, {})) node.parent.replace(img_tag) diff --git a/lib/banzai/filter/suggestion_filter.rb b/lib/banzai/filter/suggestion_filter.rb index 56a14ec073..aa1fcb1021 100644 --- a/lib/banzai/filter/suggestion_filter.rb +++ b/lib/banzai/filter/suggestion_filter.rb @@ -7,10 +7,13 @@ module Banzai # Class used for tagging elements that should be rendered TAG_CLASS = 'js-render-suggestion' + CSS = 'pre.language-suggestion > code' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + def call return doc unless suggestions_filter_enabled? - doc.search('pre.language-suggestion > code').each do |node| + doc.xpath(XPATH).each do |node| node.add_class(TAG_CLASS) end diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 731a2bb4c7..eceeb11355 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -14,8 +14,11 @@ module Banzai PARAMS_DELIMITER = ':' LANG_PARAMS_ATTR = 'data-lang-params' + CSS = 'pre:not([data-math-style]):not([data-mermaid-style]):not([data-kroki-style]) > code' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + def call - doc.search('pre:not([data-math-style]):not([data-mermaid-style]):not([data-kroki-style]) > code').each do |node| + doc.xpath(XPATH).each do |node| highlight_node(node) end diff --git a/lib/banzai/filter/table_of_contents_filter.rb b/lib/banzai/filter/table_of_contents_filter.rb index b362607aed..13ca9cde56 100644 --- a/lib/banzai/filter/table_of_contents_filter.rb +++ b/lib/banzai/filter/table_of_contents_filter.rb @@ -19,6 +19,9 @@ module Banzai class TableOfContentsFilter < HTML::Pipeline::Filter include Gitlab::Utils::Markdown + CSS = 'h1, h2, h3, h4, h5, h6' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + def call return doc if context[:no_header_anchors] @@ -27,7 +30,7 @@ module Banzai headers = Hash.new(0) header_root = current_header = HeaderNode.new - doc.css('h1, h2, h3, h4, h5, h6').each do |node| + doc.xpath(XPATH).each do |node| if header_content = node.children.first id = string_to_anchor(node.text) diff --git a/lib/banzai/filter/truncate_source_filter.rb b/lib/banzai/filter/truncate_source_filter.rb index 44f88b253d..a21d4a4429 100644 --- a/lib/banzai/filter/truncate_source_filter.rb +++ b/lib/banzai/filter/truncate_source_filter.rb @@ -3,12 +3,29 @@ module Banzai module Filter class TruncateSourceFilter < HTML::Pipeline::TextFilter + CHARACTER_COUNT_LIMIT = 1.megabyte + USER_MSG_LIMIT = 10_000 + def call - return text unless context.key?(:limit) + # don't truncate if it's a :blob and no limit is set + return text if context[:text_source] == :blob && !context.key?(:limit) + + limit = context[:limit] || CHARACTER_COUNT_LIMIT + + # no sense in allowing `truncate_bytes` to duplicate a large + # string unless it's too big + return text if text.bytesize <= limit # Use three dots instead of the ellipsis Unicode character because # some clients show the raw Unicode value in the merge commit. - text.truncate_bytes(context[:limit], omission: '...') + trunc = text.truncate_bytes(limit, omission: '...') + + # allows us to indicate to the user that what they see is a truncated copy + if limit > USER_MSG_LIMIT + trunc.prepend("_The text is longer than #{limit} characters and has been visually truncated._\n\n") + end + + trunc end end end diff --git a/lib/banzai/filter/wiki_link_filter.rb b/lib/banzai/filter/wiki_link_filter.rb index 44f13612fd..2b95d87ff8 100644 --- a/lib/banzai/filter/wiki_link_filter.rb +++ b/lib/banzai/filter/wiki_link_filter.rb @@ -10,14 +10,21 @@ module Banzai class WikiLinkFilter < HTML::Pipeline::Filter include Gitlab::Utils::SanitizeNodeLink + CSS_A = 'a:not(.gfm)' + XPATH_A = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_A).freeze + CSS_VA = 'video, audio' + XPATH_VA = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_VA).freeze + CSS_IMG = 'img' + XPATH_IMG = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_IMG).freeze + def call return doc unless wiki? - doc.search('a:not(.gfm)').each { |el| process_link(el.attribute('href'), el) } + doc.xpath(XPATH_A).each { |el| process_link(el.attribute('href'), el) } - doc.search('video, audio').each { |el| process_link(el.attribute('src'), el) } + doc.xpath(XPATH_VA).each { |el| process_link(el.attribute('src'), el) } - doc.search('img').each do |el| + doc.xpath(XPATH_IMG).each do |el| attr = el.attribute('data-src') || el.attribute('src') process_link(attr, el) diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 1f5cce249d..44561ee6d4 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -84,7 +84,7 @@ module Gitlab Gitlab::Auth::UniqueIpsLimiter.limit_user! do user = User.by_login(login) - break if user && !user.can?(:log_in) + break if user && !can_user_login_with_non_expired_password?(user) authenticators = [] @@ -182,7 +182,7 @@ module Gitlab if valid_oauth_token?(token) user = User.id_in(token.resource_owner_id).first - return unless user&.can?(:log_in) + return unless user && can_user_login_with_non_expired_password?(user) Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities) end @@ -200,7 +200,7 @@ module Gitlab return if project && token.user.project_bot? && !project.bots.include?(token.user) - if token.user.can?(:log_in) || token.user.project_bot? + if can_user_login_with_non_expired_password?(token.user) || token.user.project_bot? Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes)) end end @@ -285,7 +285,7 @@ module Gitlab return unless build.project.builds_enabled? if build.user - return unless build.user.can?(:log_in) || (build.user.project_bot? && build.project.bots&.include?(build.user)) + return unless can_user_login_with_non_expired_password?(build.user) || (build.user.project_bot? && build.project.bots&.include?(build.user)) # If user is assigned to build, use restricted credentials of user Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities) @@ -380,6 +380,10 @@ module Gitlab user.increment_failed_attempts! end + + def can_user_login_with_non_expired_password?(user) + user.can?(:log_in) && !user.password_expired? + end end end end diff --git a/lib/gitlab/auth/user_access_denied_reason.rb b/lib/gitlab/auth/user_access_denied_reason.rb index 36b54ba2e4..6639000dba 100644 --- a/lib/gitlab/auth/user_access_denied_reason.rb +++ b/lib/gitlab/auth/user_access_denied_reason.rb @@ -23,6 +23,9 @@ module Gitlab "Your primary email address is not confirmed. "\ "Please check your inbox for the confirmation instructions. "\ "In case the link is expired, you can request a new confirmation email at #{Rails.application.routes.url_helpers.new_user_confirmation_url}" + when :password_expired + "Your password expired. "\ + "Please access GitLab from a web browser to update your password." else "Your account has been blocked." end @@ -41,6 +44,8 @@ module Gitlab :deactivated elsif !@user.confirmed? :unconfirmed + elsif @user.password_expired? + :password_expired else :blocked end diff --git a/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group.rb b/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group.rb new file mode 100644 index 0000000000..f5ba9e6333 --- /dev/null +++ b/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + class UpdateUsersWhereTwoFactorAuthRequiredFromGroup # rubocop:disable Metrics/ClassLength + def perform(start_id, stop_id) + ActiveRecord::Base.connection.execute <<~SQL + UPDATE + users + SET + require_two_factor_authentication_from_group = TRUE + WHERE + users.id BETWEEN #{start_id} + AND #{stop_id} + AND users.require_two_factor_authentication_from_group = FALSE + AND users.id IN ( + SELECT + DISTINCT users_groups_query.user_id + FROM + ( + SELECT + users.id AS user_id, + members.source_id AS group_ids + FROM + users + LEFT JOIN members ON members.source_type = 'Namespace' + AND members.requested_at IS NULL + AND members.user_id = users.id + AND members.type = 'GroupMember' + WHERE + users.require_two_factor_authentication_from_group = FALSE + AND users.id BETWEEN #{start_id} + AND #{stop_id}) AS users_groups_query + INNER JOIN LATERAL ( + WITH RECURSIVE "base_and_ancestors" AS ( + ( + SELECT + "namespaces"."type", + "namespaces"."id", + "namespaces"."parent_id", + "namespaces"."require_two_factor_authentication" + FROM + "namespaces" + WHERE + "namespaces"."type" = 'Group' + AND "namespaces"."id" = users_groups_query.group_ids + ) + UNION + ( + SELECT + "namespaces"."type", + "namespaces"."id", + "namespaces"."parent_id", + "namespaces"."require_two_factor_authentication" + FROM + "namespaces", + "base_and_ancestors" + WHERE + "namespaces"."type" = 'Group' + AND "namespaces"."id" = "base_and_ancestors"."parent_id" + ) + ), + "base_and_descendants" AS ( + ( + SELECT + "namespaces"."type", + "namespaces"."id", + "namespaces"."parent_id", + "namespaces"."require_two_factor_authentication" + FROM + "namespaces" + WHERE + "namespaces"."type" = 'Group' + AND "namespaces"."id" = users_groups_query.group_ids + ) + UNION + ( + SELECT + "namespaces"."type", + "namespaces"."id", + "namespaces"."parent_id", + "namespaces"."require_two_factor_authentication" + FROM + "namespaces", + "base_and_descendants" + WHERE + "namespaces"."type" = 'Group' + AND "namespaces"."parent_id" = "base_and_descendants"."id" + ) + ) + SELECT + "namespaces".* + FROM + ( + ( + SELECT + "namespaces"."type", + "namespaces"."id", + "namespaces"."parent_id", + "namespaces"."require_two_factor_authentication" + FROM + "base_and_ancestors" AS "namespaces" + WHERE + "namespaces"."type" = 'Group' + ) + UNION + ( + SELECT + "namespaces"."type", + "namespaces"."id", + "namespaces"."parent_id", + "namespaces"."require_two_factor_authentication" + FROM + "base_and_descendants" AS "namespaces" + WHERE + "namespaces"."type" = 'Group' + ) + ) namespaces + WHERE + "namespaces"."type" = 'Group' + AND "namespaces"."require_two_factor_authentication" = TRUE + ) AS hierarchy_tree ON TRUE + ); + SQL + end + end + end +end diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 55f381fcb6..510c945dd8 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -7,6 +7,10 @@ module Gitlab !signup_enabled? end + def signup_limited? + domain_allowlist.present? || email_restrictions_enabled? || require_admin_approval_after_user_signup? + end + def current_application_settings Gitlab::SafeRequestStore.fetch(:current_application_settings) { ensure_application_settings! } end diff --git a/lib/gitlab/diff/suggestions_parser.rb b/lib/gitlab/diff/suggestions_parser.rb index f3e6fc455a..6f12614711 100644 --- a/lib/gitlab/diff/suggestions_parser.rb +++ b/lib/gitlab/diff/suggestions_parser.rb @@ -6,6 +6,9 @@ module Gitlab # Matches for instance "-1", "+1" or "-1+2". SUGGESTION_CONTEXT = /^(\-(?\d+))?(\+(?\d+))?$/.freeze + CSS = 'pre.language-suggestion' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze + class << self # Returns an array of Gitlab::Diff::Suggestion which represents each # suggestion in the given text. @@ -17,7 +20,7 @@ module Gitlab no_original_data: true, suggestions_filter_enabled: supports_suggestion) doc = Nokogiri::HTML(html) - suggestion_nodes = doc.search('pre.language-suggestion') + suggestion_nodes = doc.xpath(XPATH) return [] if suggestion_nodes.empty? diff --git a/lib/gitlab/middleware/read_only/controller.rb b/lib/gitlab/middleware/read_only/controller.rb index b11ee0afc1..226ef2041b 100644 --- a/lib/gitlab/middleware/read_only/controller.rb +++ b/lib/gitlab/middleware/read_only/controller.rb @@ -83,7 +83,15 @@ module Gitlab end def route_hash - @route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {} + @route_hash ||= Rails.application.routes.recognize_path(request_url, { method: request.request_method }) rescue {} + end + + def request_url + request.url.chomp('/') + end + + def request_path + @request_path ||= request.path.chomp('/') end def relative_url @@ -100,7 +108,7 @@ module Gitlab def workhorse_passthrough_route? # Calling route_hash may be expensive. Only do it if we think there's a possible match return false unless request.post? && - request.path.end_with?('.git/git-upload-pack') + request_path.end_with?('.git/git-upload-pack') ALLOWLISTED_GIT_READ_ONLY_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) end @@ -120,14 +128,14 @@ module Gitlab # https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/repositories/lfs_api_controller.rb#L106 def lfs_batch_route? # Calling route_hash may be expensive. Only do it if we think there's a possible match - return unless request.path.end_with?('/info/lfs/objects/batch') + return unless request_path.end_with?('/info/lfs/objects/batch') ALLOWLISTED_GIT_LFS_BATCH_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) end def session_route? # Calling route_hash may be expensive. Only do it if we think there's a possible match - return false unless request.post? && request.path.end_with?('/users/sign_out', + return false unless request.post? && request_path.end_with?('/users/sign_out', '/admin/session', '/admin/session/destroy') ALLOWLISTED_SESSION_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index c1a5756664..98b3a77d09 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -197,6 +197,24 @@ module Gitlab rescue Addressable::URI::InvalidURIError, TypeError end + def removes_sensitive_data_from_url(uri_string) + uri = parse_url(uri_string) + + return unless uri + return uri_string unless uri.fragment + + stripped_params = CGI.parse(uri.fragment) + if stripped_params['access_token'] + stripped_params['access_token'] = 'filtered' + filtered_query = Addressable::URI.new + filtered_query.query_values = stripped_params + + uri.fragment = filtered_query.query + end + + uri.to_s + end + # Invert a hash, collecting all keys that map to a given value in an array. # # Unlike `Hash#invert`, where the last encountered pair wins, and which has the diff --git a/lib/gitlab/utils/nokogiri.rb b/lib/gitlab/utils/nokogiri.rb new file mode 100644 index 0000000000..4b37bb7e5e --- /dev/null +++ b/lib/gitlab/utils/nokogiri.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Gitlab + module Utils + class Nokogiri + class << self + # Use Nokogiri to convert a css selector into an xpath selector. + # Nokogiri can use css selectors with `doc.search()`. However + # for large node trees, it is _much_ slower than using xpath, + # by several orders of magnitude. + # https://gitlab.com/gitlab-org/gitlab/-/issues/329186 + def css_to_xpath(css) + xpath = ::Nokogiri::CSS.xpath_for(css) + + # Due to https://github.com/sparklemotion/nokogiri/issues/572, + # we remove the leading `//` and add `descendant-or-self::` + # in order to ensure we're searching from this node and all + # descendants. + xpath.map { |t| "descendant-or-self::#{t[2..-1]}" }.join('|') + end + end + end + end +end diff --git a/lib/gitlab/x509/signature.rb b/lib/gitlab/x509/signature.rb index edff1540cb..72bbf3d6e8 100644 --- a/lib/gitlab/x509/signature.rb +++ b/lib/gitlab/x509/signature.rb @@ -23,7 +23,7 @@ module Gitlab end def user - User.find_by_any_email(@email) + strong_memoize(:user) { User.find_by_any_email(@email) } end def verified_signature @@ -31,9 +31,13 @@ module Gitlab end def verification_status - return :unverified if x509_certificate.nil? || x509_certificate.revoked? + return :unverified if + x509_certificate.nil? || + x509_certificate.revoked? || + !verified_signature || + user.nil? - if verified_signature && certificate_email == @email + if user.verified_emails.include?(@email) && certificate_email == @email :verified else :unverified diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 35d38f13bf..35d599dfb6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26087,6 +26087,9 @@ msgstr "" msgid "Redirect to SAML provider to test configuration" msgstr "" +msgid "Redirecting" +msgstr "" + msgid "Redis" msgstr "" diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 21124299b2..0e25f6a96d 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -70,12 +70,29 @@ RSpec.describe Oauth::AuthorizationsController do describe 'GET #new' do subject { get :new, params: params } - include_examples 'OAuth Authorizations require confirmed user' include_examples "Implicit grant can't be used in confidential application" context 'when the user is confirmed' do let(:confirmed_at) { 1.hour.ago } + context 'when there is already an access token for the application with a matching scope' do + before do + scopes = Doorkeeper::OAuth::Scopes.from_string('api') + + allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes) + + create(:oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes) + end + + it 'authorizes the request and shows the user a page that redirects' do + subject + + expect(request.session['user_return_to']).to be_nil + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/redirect') + end + end + context 'without valid params' do it 'returns 200 code and renders error view' do get :new @@ -102,7 +119,8 @@ RSpec.describe Oauth::AuthorizationsController do subject expect(request.session['user_return_to']).to be_nil - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/redirect') end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 753223c5a4..4a1d01f0e8 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -290,6 +290,39 @@ RSpec.describe Projects::PipelinesController do end end + describe 'GET #show' do + render_views + + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + subject { get_pipeline_html } + + def get_pipeline_html + get :show, params: { namespace_id: project.namespace, project_id: project, id: pipeline }, format: :html + end + + def create_build_with_artifacts(stage, stage_idx, name) + create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name) + end + + before do + create_build_with_artifacts('build', 0, 'job1') + create_build_with_artifacts('build', 0, 'job2') + end + + it 'avoids N+1 database queries', :request_store do + get_pipeline_html + + control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count + expect(response).to have_gitlab_http_status(:ok) + + create_build_with_artifacts('build', 0, 'job3') + + expect { get_pipeline_html }.not_to exceed_query_limit(control_count) + expect(response).to have_gitlab_http_status(:ok) + end + end + describe 'GET show.json' do let(:pipeline) { create(:ci_pipeline, project: project) } diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index e60d9d6ab6..1d7099ba44 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -394,6 +394,25 @@ RSpec.describe 'Login' do gitlab_sign_in(user) end + + context 'when the users password is expired' do + before do + user.update!(password_expires_at: Time.parse('2018-05-08 11:29:46 UTC')) + end + + it 'asks for a new password' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + visit new_user_session_path + + fill_in 'user_login', with: user.email + fill_in 'user_password', with: '12345678' + click_button 'Sign in' + + expect(current_path).to eq(new_profile_password_path) + end + end end context 'with invalid username and password' do diff --git a/spec/frontend/notebook/cells/markdown_spec.js b/spec/frontend/notebook/cells/markdown_spec.js index 219d74595b..5c6371530d 100644 --- a/spec/frontend/notebook/cells/markdown_spec.js +++ b/spec/frontend/notebook/cells/markdown_spec.js @@ -39,7 +39,7 @@ describe('Markdown component', () => { expect(vm.$el.querySelector('.markdown h1')).not.toBeNull(); }); - it('sanitizes output', async () => { + it('sanitizes Markdown output', async () => { Object.assign(cell, { source: [ '[XSS](data:text/html;base64,PHNjcmlwdD5hbGVydChkb2N1bWVudC5kb21haW4pPC9zY3JpcHQ+Cg==)\n', @@ -50,6 +50,17 @@ describe('Markdown component', () => { expect(vm.$el.querySelector('a').getAttribute('href')).toBeNull(); }); + it('sanitizes HTML', async () => { + const findLink = () => vm.$el.querySelector('.xss-link'); + Object.assign(cell, { + source: ['XSS\n'], + }); + + await vm.$nextTick(); + expect(findLink().getAttribute('data-remote')).toBe(null); + expect(findLink().getAttribute('data-type')).toBe(null); + }); + describe('katex', () => { beforeEach(() => { json = getJSONFixture('blob/notebook/math.json'); diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index 00a59f037e..e946857ac7 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -418,6 +418,13 @@ FooBar describe '#markup' do let(:content) { 'Noël' } + it 'sets the :text_source to :blob in the context' do + context = {} + helper.markup('foo.md', content, context) + + expect(context).to include(text_source: :blob) + end + it 'preserves encoding' do expect(content.encoding.name).to eq('UTF-8') expect(helper.markup('foo.rst', content).encoding.name).to eq('UTF-8') diff --git a/spec/initializers/lograge_spec.rb b/spec/initializers/lograge_spec.rb index abb1673bb8..421f6373ef 100644 --- a/spec/initializers/lograge_spec.rb +++ b/spec/initializers/lograge_spec.rb @@ -173,6 +173,27 @@ RSpec.describe 'lograge', type: :request do end end + describe 'with access token in url' do + before do + event.payload[:location] = 'http://example.com/auth.html#access_token=secret_token&token_type=Bearer' + end + + it 'strips location from sensitive information' do + subscriber.redirect_to(event) + subscriber.process_action(event) + + expect(log_data['location']).not_to include('secret_token') + expect(log_data['location']).to include('filtered') + end + + it 'leaves non-sensitive information from location' do + subscriber.redirect_to(event) + subscriber.process_action(event) + + expect(log_data['location']).to include('&token_type=Bearer') + end + end + context 'with db payload' do context 'when RequestStore is enabled', :request_store do it 'includes db counters' do diff --git a/spec/lib/banzai/filter/truncate_source_filter_spec.rb b/spec/lib/banzai/filter/truncate_source_filter_spec.rb index d5eb8b738b..8970aa1d38 100644 --- a/spec/lib/banzai/filter/truncate_source_filter_spec.rb +++ b/spec/lib/banzai/filter/truncate_source_filter_spec.rb @@ -8,24 +8,68 @@ RSpec.describe Banzai::Filter::TruncateSourceFilter do let(:short_text) { 'foo' * 10 } let(:long_text) { ([short_text] * 10).join(' ') } - it 'does nothing when limit is unspecified' do - output = filter(long_text) - - expect(output).to eq(long_text) + before do + stub_const("#{described_class}::CHARACTER_COUNT_LIMIT", 50) + stub_const("#{described_class}::USER_MSG_LIMIT", 20) end - it 'does nothing to a short-enough text' do - output = filter(short_text, limit: short_text.bytesize) + context 'when markdown belongs to a blob' do + it 'does nothing when limit is unspecified' do + output = filter(long_text, text_source: :blob) - expect(output).to eq(short_text) + expect(output).to eq(long_text) + end + + it 'truncates normally when limit specified' do + truncated = 'foofoof...' + + output = filter(long_text, text_source: :blob, limit: 10) + + expect(output).to eq(truncated) + end end - it 'truncates UTF-8 text by bytes, on a character boundary' do - utf8_text = '日本語の文字が大きい' - truncated = '日...' + context 'when markdown belongs to a field (non-blob)' do + it 'does nothing when limit is greater' do + output = filter(long_text, limit: 1.megabyte) - expect(filter(utf8_text, limit: truncated.bytesize)).to eq(truncated) - expect(filter(utf8_text, limit: utf8_text.bytesize)).to eq(utf8_text) - expect(filter(utf8_text, limit: utf8_text.mb_chars.size)).not_to eq(utf8_text) + expect(output).to eq(long_text) + end + + it 'truncates to the default when limit is unspecified' do + stub_const("#{described_class}::USER_MSG_LIMIT", 200) + truncated = 'foofoofoofoofoofoofoofoofoofoo foofoofoofoofoof...' + + output = filter(long_text) + + expect(output).to eq(truncated) + end + + it 'prepends the user message' do + truncated = <<~TEXT + _The text is longer than 50 characters and has been visually truncated._ + + foofoofoofoofoofoofoofoofoofoo foofoofoofoofoof... + TEXT + + output = filter(long_text) + + expect(output).to eq(truncated.strip) + end + + it 'does nothing to a short-enough text' do + output = filter(short_text, limit: short_text.bytesize) + + expect(output).to eq(short_text) + end + + it 'truncates UTF-8 text by bytes, on a character boundary' do + utf8_text = '日本語の文字が大きい' + truncated = '日...' + + expect(filter(utf8_text, limit: truncated.bytesize)).to eq(truncated) + expect(filter(utf8_text, limit: utf8_text.bytesize)).to eq(utf8_text) + expect(filter(utf8_text, limit: utf8_text.mb_chars.size)).not_to eq(utf8_text) + end end end diff --git a/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb b/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb index d3c6cde559..102d6fba97 100644 --- a/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb +++ b/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb @@ -57,5 +57,13 @@ RSpec.describe Gitlab::Auth::UserAccessDeniedReason do it { is_expected.to eq('Your account is pending approval from your administrator and hence blocked.') } end + + context 'when the user has expired password' do + before do + user.update!(password_expires_at: 2.days.ago) + end + + it { is_expected.to eq('Your password expired. Please access GitLab from a web browser to update your password.') } + end end end diff --git a/spec/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group_spec.rb b/spec/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group_spec.rb new file mode 100644 index 0000000000..e14328b615 --- /dev/null +++ b/spec/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::UpdateUsersWhereTwoFactorAuthRequiredFromGroup, :migration, schema: 20210519154058 do + include MigrationHelpers::NamespacesHelpers + + let(:group_with_2fa_parent) { create_namespace('parent', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: true) } + let(:group_with_2fa_child) { create_namespace('child', Gitlab::VisibilityLevel::PRIVATE, parent_id: group_with_2fa_parent.id) } + let(:members_table) { table(:members) } + let(:users_table) { table(:users) } + + subject { described_class.new } + + describe '#perform' do + context 'with group members' do + let(:user_1) { create_user('user@example.com') } + let!(:member) { create_group_member(user_1, group_with_2fa_parent) } + let!(:user_without_group) { create_user('user_without@example.com') } + let(:user_other) { create_user('user_other@example.com') } + let!(:member_other) { create_group_member(user_other, group_with_2fa_parent) } + + it 'updates user when user should be required to establish two factor authentication' do + subject.perform(user_1.id, user_without_group.id) + + expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true) + end + + it 'does not update user who is not in current batch' do + subject.perform(user_1.id, user_without_group.id) + + expect(user_other.reload.require_two_factor_authentication_from_group).to eq(false) + end + + it 'updates all users in current batch' do + subject.perform(user_1.id, user_other.id) + + expect(user_other.reload.require_two_factor_authentication_from_group).to eq(true) + end + + it 'updates user when user is member of group in which parent group requires two factor authentication' do + member.destroy! + + subgroup = create_namespace('subgroup', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: false, parent_id: group_with_2fa_child.id) + create_group_member(user_1, subgroup) + + subject.perform(user_1.id, user_other.id) + + expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true) + end + + it 'updates user when user is member of a group and the subgroup requires two factor authentication' do + member.destroy! + + parent = create_namespace('other_parent', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: false) + create_namespace('other_subgroup', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: true, parent_id: parent.id) + create_group_member(user_1, parent) + + subject.perform(user_1.id, user_other.id) + + expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true) + end + + it 'does not update user when not a member of a group that requires two factor authentication' do + member_other.destroy! + + other_group = create_namespace('other_group', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: false) + create_group_member(user_other, other_group) + + subject.perform(user_1.id, user_other.id) + + expect(user_other.reload.require_two_factor_authentication_from_group).to eq(false) + end + end + end + + def create_user(email, require_2fa: false) + users_table.create!(email: email, projects_limit: 10, require_two_factor_authentication_from_group: require_2fa) + end + + def create_group_member(user, group) + members_table.create!(user_id: user.id, source_id: group.id, access_level: GroupMember::MAINTAINER, source_type: "Namespace", type: "GroupMember", notification_level: 3) + end +end diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index f5cb1987c5..a5ab1047a4 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -24,6 +24,42 @@ RSpec.describe Gitlab::CurrentSettings do end end + describe '.signup_limited?' do + subject { described_class.signup_limited? } + + context 'when there are allowed domains' do + before do + create(:application_setting, domain_allowlist: ['www.gitlab.com']) + end + + it { is_expected.to be_truthy } + end + + context 'when there are email restrictions' do + before do + create(:application_setting, email_restrictions_enabled: true) + end + + it { is_expected.to be_truthy } + end + + context 'when the admin has to approve signups' do + before do + create(:application_setting, require_admin_approval_after_user_signup: true) + end + + it { is_expected.to be_truthy } + end + + context 'when there are no restrictions' do + before do + create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false) + end + + it { is_expected.to be_falsey } + end + end + describe '.signup_disabled?' do subject { described_class.signup_disabled? } diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 9a1ecfe645..ff07696992 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -433,6 +433,13 @@ RSpec.describe Gitlab::GitAccess do expect { pull_access_check }.to raise_forbidden("Your account has been deactivated by your administrator. Please log back in from a web browser to reactivate your account at #{Gitlab.config.gitlab.url}") end + it 'disallows users with expired password to pull' do + project.add_maintainer(user) + user.update!(password_expires_at: 2.minutes.ago) + + expect { pull_access_check }.to raise_forbidden("Your password expired. Please access GitLab from a web browser to update your password.") + end + context 'when the project repository does not exist' do before do project.add_guest(user) @@ -969,6 +976,13 @@ RSpec.describe Gitlab::GitAccess do expect { push_access_check }.to raise_forbidden("Your account has been deactivated by your administrator. Please log back in from a web browser to reactivate your account at #{Gitlab.config.gitlab.url}") end + it 'disallows users with expired password to push' do + project.add_maintainer(user) + user.update!(password_expires_at: 2.minutes.ago) + + expect { push_access_check }.to raise_forbidden("Your password expired. Please access GitLab from a web browser to update your password.") + end + it 'cleans up the files' do expect(project.repository).to receive(:clean_stale_repository_files).and_call_original expect { push_access_check }.not_to raise_error diff --git a/spec/lib/gitlab/utils/nokogiri_spec.rb b/spec/lib/gitlab/utils/nokogiri_spec.rb new file mode 100644 index 0000000000..90f137f53c --- /dev/null +++ b/spec/lib/gitlab/utils/nokogiri_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Utils::Nokogiri do + describe '#css_to_xpath' do + using RSpec::Parameterized::TableSyntax + + where(:css, :xpath) do + 'img' | "descendant-or-self::img" + 'a.gfm' | "descendant-or-self::a[contains(concat(' ',normalize-space(@class),' '),' gfm ')]" + 'a:not(.gfm)' | "descendant-or-self::a[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]" + 'video, audio' | "descendant-or-self::video|descendant-or-self::audio" + '[data-math-style]' | "descendant-or-self::*[@data-math-style]" + '[data-mermaid-style]' | "descendant-or-self::*[@data-mermaid-style]" + '.js-render-metrics' | "descendant-or-self::*[contains(concat(' ',normalize-space(@class),' '),' js-render-metrics ')]" + 'h1, h2, h3, h4, h5, h6' | "descendant-or-self::h1|descendant-or-self::h2|descendant-or-self::h3|descendant-or-self::h4|descendant-or-self::h5|descendant-or-self::h6" + 'pre.code.language-math' | "descendant-or-self::pre[contains(concat(' ',normalize-space(@class),' '),' code ') and contains(concat(' ',normalize-space(@class),' '),' language-math ')]" + 'pre > code[lang="plantuml"]' | "descendant-or-self::pre/code[@lang=\"plantuml\"]" + 'pre[lang="mermaid"] > code' | "descendant-or-self::pre[@lang=\"mermaid\"]/code" + 'pre.language-suggestion' | "descendant-or-self::pre[contains(concat(' ',normalize-space(@class),' '),' language-suggestion ')]" + 'pre.language-suggestion > code' | "descendant-or-self::pre[contains(concat(' ',normalize-space(@class),' '),' language-suggestion ')]/code" + 'a.gfm[data-reference-type="user"]' | "descendant-or-self::a[contains(concat(' ',normalize-space(@class),' '),' gfm ') and @data-reference-type=\"user\"]" + 'a:not(.gfm), img:not(.gfm), video:not(.gfm), audio:not(.gfm)' | "descendant-or-self::a[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]|descendant-or-self::img[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]|descendant-or-self::video[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]|descendant-or-self::audio[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]" + 'pre:not([data-math-style]):not([data-mermaid-style]):not([data-kroki-style]) > code' | "descendant-or-self::pre[not(@data-math-style) and not(@data-mermaid-style) and not(@data-kroki-style)]/code" + end + + with_them do + it 'generates the xpath' do + expect(described_class.css_to_xpath(css)).to eq xpath + end + end + end +end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index 11dba610fa..a7ccce0aaa 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -417,6 +417,29 @@ RSpec.describe Gitlab::Utils do end end + describe '.removes_sensitive_data_from_url' do + it 'returns string object' do + expect(described_class.removes_sensitive_data_from_url('http://gitlab.com')).to be_instance_of(String) + end + + it 'returns nil when URI cannot be parsed' do + expect(described_class.removes_sensitive_data_from_url('://gitlab.com')).to be nil + end + + it 'returns nil with invalid parameter' do + expect(described_class.removes_sensitive_data_from_url(1)).to be nil + end + + it 'returns string with filtered access_token param' do + expect(described_class.removes_sensitive_data_from_url('http://gitlab.com/auth.html#access_token=secret_token')).to eq('http://gitlab.com/auth.html#access_token=filtered') + end + + it 'returns string with filtered access_token param but other params preserved' do + expect(described_class.removes_sensitive_data_from_url('http://gitlab.com/auth.html#access_token=secret_token&token_type=Bearer&state=test')) + .to include('&token_type=Bearer', '&state=test') + end + end + describe 'multiple_key_invert' do it 'invert keys with array values' do hash = { diff --git a/spec/lib/gitlab/x509/signature_spec.rb b/spec/lib/gitlab/x509/signature_spec.rb index 2ac9c1f3a3..7ba15faf91 100644 --- a/spec/lib/gitlab/x509/signature_spec.rb +++ b/spec/lib/gitlab/x509/signature_spec.rb @@ -12,20 +12,30 @@ RSpec.describe Gitlab::X509::Signature do end shared_examples "a verified signature" do - it 'returns a verified signature if email does match' do - signature = described_class.new( + let_it_be(:user) { create(:user, email: X509Helpers::User1.certificate_email) } + + subject(:signature) do + described_class.new( X509Helpers::User1.signed_commit_signature, X509Helpers::User1.signed_commit_base_data, X509Helpers::User1.certificate_email, X509Helpers::User1.signed_commit_time ) + end + it 'returns a verified signature if email does match' do expect(signature.x509_certificate).to have_attributes(certificate_attributes) expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) expect(signature.verified_signature).to be_truthy expect(signature.verification_status).to eq(:verified) end + it "returns an unverified signature if the email matches but isn't confirmed" do + user.update!(confirmed_at: nil) + + expect(signature.verification_status).to eq(:unverified) + end + it 'returns an unverified signature if email does not match' do signature = described_class.new( X509Helpers::User1.signed_commit_signature, @@ -55,13 +65,6 @@ RSpec.describe Gitlab::X509::Signature do end it 'returns an unverified signature if certificate is revoked' do - signature = described_class.new( - X509Helpers::User1.signed_commit_signature, - X509Helpers::User1.signed_commit_base_data, - X509Helpers::User1.certificate_email, - X509Helpers::User1.signed_commit_time - ) - expect(signature.verification_status).to eq(:verified) signature.x509_certificate.revoked! @@ -253,23 +256,25 @@ RSpec.describe Gitlab::X509::Signature do end describe '#user' do - signature = described_class.new( - X509Helpers::User1.signed_tag_signature, - X509Helpers::User1.signed_tag_base_data, - X509Helpers::User1.certificate_email, - X509Helpers::User1.signed_commit_time - ) + subject do + described_class.new( + X509Helpers::User1.signed_tag_signature, + X509Helpers::User1.signed_tag_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ).user + end context 'if email is assigned to a user' do let!(:user) { create(:user, email: X509Helpers::User1.certificate_email) } it 'returns user' do - expect(signature.user).to eq(user) + is_expected.to eq(user) end end it 'if email is not assigned to a user, return nil' do - expect(signature.user).to be_nil + is_expected.to be_nil end end @@ -292,6 +297,17 @@ RSpec.describe Gitlab::X509::Signature do end context 'verified signature' do + let_it_be(:user) { create(:user, email: X509Helpers::User1.certificate_email) } + + subject(:signature) do + described_class.new( + X509Helpers::User1.signed_tag_signature, + X509Helpers::User1.signed_tag_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + end + context 'with trusted certificate store' do before do store = OpenSSL::X509::Store.new @@ -301,19 +317,18 @@ RSpec.describe Gitlab::X509::Signature do end it 'returns a verified signature if email does match' do - signature = described_class.new( - X509Helpers::User1.signed_tag_signature, - X509Helpers::User1.signed_tag_base_data, - X509Helpers::User1.certificate_email, - X509Helpers::User1.signed_commit_time - ) - expect(signature.x509_certificate).to have_attributes(certificate_attributes) expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) expect(signature.verified_signature).to be_truthy expect(signature.verification_status).to eq(:verified) end + it "returns an unverified signature if the email matches but isn't confirmed" do + user.update!(confirmed_at: nil) + + expect(signature.verification_status).to eq(:unverified) + end + it 'returns an unverified signature if email does not match' do signature = described_class.new( X509Helpers::User1.signed_tag_signature, @@ -343,13 +358,6 @@ RSpec.describe Gitlab::X509::Signature do end it 'returns an unverified signature if certificate is revoked' do - signature = described_class.new( - X509Helpers::User1.signed_tag_signature, - X509Helpers::User1.signed_tag_base_data, - X509Helpers::User1.certificate_email, - X509Helpers::User1.signed_commit_time - ) - expect(signature.verification_status).to eq(:verified) signature.x509_certificate.revoked! @@ -368,13 +376,6 @@ RSpec.describe Gitlab::X509::Signature do end it 'returns an unverified signature' do - signature = described_class.new( - X509Helpers::User1.signed_tag_signature, - X509Helpers::User1.signed_tag_base_data, - X509Helpers::User1.certificate_email, - X509Helpers::User1.signed_commit_time - ) - expect(signature.x509_certificate).to have_attributes(certificate_attributes) expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) expect(signature.verified_signature).to be_falsey diff --git a/spec/migrations/schedule_update_users_where_two_factor_auth_required_from_group_spec.rb b/spec/migrations/schedule_update_users_where_two_factor_auth_required_from_group_spec.rb new file mode 100644 index 0000000000..cec141cacc --- /dev/null +++ b/spec/migrations/schedule_update_users_where_two_factor_auth_required_from_group_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb') + +RSpec.describe ScheduleUpdateUsersWhereTwoFactorAuthRequiredFromGroup do + let(:users) { table(:users) } + let!(:user_1) { users.create!(require_two_factor_authentication_from_group: false, name: "user1", email: "user1@example.com", projects_limit: 1) } + let!(:user_2) { users.create!(require_two_factor_authentication_from_group: true, name: "user2", email: "user2@example.com", projects_limit: 1) } + let!(:user_3) { users.create!(require_two_factor_authentication_from_group: false, name: "user3", email: "user3@example.com", projects_limit: 1) } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + end + + it 'schedules jobs for users that do not require two factor authentication' do + Sidekiq::Testing.fake! do + freeze_time do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_delayed_migration( + 2.minutes, user_1.id, user_1.id) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration( + 4.minutes, user_3.id, user_3.id) + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index e677f5558f..bbbc5d08c0 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -239,6 +239,14 @@ RSpec.describe GlobalPolicy do it { is_expected.not_to be_allowed(:access_api) } end + context 'user with expired password' do + before do + current_user.update!(password_expires_at: 2.minutes.ago) + end + + it { is_expected.not_to be_allowed(:access_api) } + end + context 'when terms are enforced' do before do enforce_terms @@ -418,6 +426,14 @@ RSpec.describe GlobalPolicy do it { is_expected.not_to be_allowed(:access_git) } end + + context 'user with expired password' do + before do + current_user.update!(password_expires_at: 2.minutes.ago) + end + + it { is_expected.not_to be_allowed(:access_git) } + end end describe 'read instance metadata' do @@ -494,6 +510,14 @@ RSpec.describe GlobalPolicy do it { is_expected.not_to be_allowed(:use_slash_commands) } end + + context 'user with expired password' do + before do + current_user.update!(password_expires_at: 2.minutes.ago) + end + + it { is_expected.not_to be_allowed(:use_slash_commands) } + end end describe 'create_snippet' do diff --git a/spec/presenters/ci/stage_presenter_spec.rb b/spec/presenters/ci/stage_presenter_spec.rb new file mode 100644 index 0000000000..368f03b015 --- /dev/null +++ b/spec/presenters/ci/stage_presenter_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::StagePresenter do + let(:stage) { create(:ci_stage) } + let(:presenter) { described_class.new(stage) } + + let!(:build) { create(:ci_build, :tags, :artifacts, pipeline: stage.pipeline, stage: stage.name) } + let!(:retried_build) { create(:ci_build, :tags, :artifacts, :retried, pipeline: stage.pipeline, stage: stage.name) } + + before do + create(:generic_commit_status, pipeline: stage.pipeline, stage: stage.name) + end + + shared_examples 'preloaded associations for CI status' do + it 'preloads project' do + expect(presented_stage.association(:project)).to be_loaded + end + + it 'preloads build pipeline' do + expect(presented_stage.association(:pipeline)).to be_loaded + end + + it 'preloads build tags' do + expect(presented_stage.association(:tags)).to be_loaded + end + + it 'preloads build artifacts archive' do + expect(presented_stage.association(:job_artifacts_archive)).to be_loaded + end + + it 'preloads build artifacts metadata' do + expect(presented_stage.association(:metadata)).to be_loaded + end + end + + describe '#latest_ordered_statuses' do + subject(:presented_stage) { presenter.latest_ordered_statuses.second } + + it_behaves_like 'preloaded associations for CI status' + end + + describe '#retried_ordered_statuses' do + subject(:presented_stage) { presenter.retried_ordered_statuses.first } + + it_behaves_like 'preloaded associations for CI status' + end +end diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index f26236e025..57aa0f3619 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -27,9 +27,10 @@ RSpec.describe API::Lint do end end - context 'when signup settings are enabled' do + context 'when signup is enabled and not limited' do before do Gitlab::CurrentSettings.signup_enabled = true + stub_application_setting(domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false) end context 'when unauthenticated' do @@ -50,6 +51,31 @@ RSpec.describe API::Lint do end end + context 'when limited signup is enabled' do + before do + stub_application_setting(domain_allowlist: ['www.gitlab.com']) + Gitlab::CurrentSettings.signup_enabled = true + end + + context 'when unauthenticated' do + it 'returns unauthorized' do + post api('/ci/lint'), params: { content: 'content' } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authenticated' do + let_it_be(:api_user) { create(:user) } + + it 'returns authentication success' do + post api('/ci/lint', api_user), params: { content: 'content' } + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + context 'when authenticated' do let_it_be(:api_user) { create(:user) } diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index a1e28c1876..279c65fc2f 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -35,6 +35,26 @@ RSpec.describe 'Git HTTP requests' do expect(response.header['WWW-Authenticate']).to start_with('Basic ') end end + + context "when password is expired" do + it "responds to downloads with status 401 Unauthorized" do + user.update!(password_expires_at: 2.days.ago) + + download(path, user: user.username, password: user.password) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + context "when user is blocked" do + let(:user) { create(:user, :blocked) } + + it "responds to downloads with status 401 Unauthorized" do + download(path, user: user.username, password: user.password) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end end context "when authentication succeeds" do @@ -75,6 +95,15 @@ RSpec.describe 'Git HTTP requests' do expect(response.header['WWW-Authenticate']).to start_with('Basic ') end end + + context "when password is expired" do + it "responds to uploads with status 401 Unauthorized" do + user.update!(password_expires_at: 2.days.ago) + upload(path, user: user.username, password: user.password) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end end context "when authentication succeeds" do @@ -576,6 +605,16 @@ RSpec.describe 'Git HTTP requests' do it_behaves_like 'pulls are allowed' it_behaves_like 'pushes are allowed' + + context "when password is expired" do + it "responds to downloads with status 401 unauthorized" do + user.update!(password_expires_at: 2.days.ago) + + download(path, **env) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end end context 'when user has 2FA enabled' do @@ -649,6 +688,18 @@ RSpec.describe 'Git HTTP requests' do expect(response).to have_gitlab_http_status(:ok) end end + + context "when password is expired" do + it "responds to uploads with status 401 unauthorized" do + user.update!(password_expires_at: 2.days.ago) + + write_access_token = create(:personal_access_token, user: user, scopes: [:write_repository]) + + upload(path, user: user.username, password: write_access_token.token) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end end end @@ -860,6 +911,16 @@ RSpec.describe 'Git HTTP requests' do expect(response).to have_gitlab_http_status(:not_found) end + + context 'when users password is expired' do + it 'rejects pulls with 401 unauthorized' do + user.update!(password_expires_at: 2.days.ago) + + download(path, user: 'gitlab-ci-token', password: build.token) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end end end end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 4e18c9cb4c..0e3a025263 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -346,9 +346,7 @@ RSpec.describe 'Git LFS API and storage' do let_it_be(:user) { create(:user, password_expires_at: 1.minute.ago)} let(:role) { :reporter} - # TODO: This should return a 404 response - # https://gitlab.com/gitlab-org/gitlab/-/issues/292006 - it_behaves_like 'LFS http 200 response' + it_behaves_like 'LFS http 401 response' end context 'when user is blocked' do diff --git a/spec/support/shared_examples/lib/gitlab/middleware/read_only_gitlab_instance_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/middleware/read_only_gitlab_instance_shared_examples.rb index 5b3d30df73..0a07a56d41 100644 --- a/spec/support/shared_examples/lib/gitlab/middleware/read_only_gitlab_instance_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/middleware/read_only_gitlab_instance_shared_examples.rb @@ -70,6 +70,14 @@ RSpec.shared_examples 'write access for a read-only GitLab instance' do expect(subject).not_to disallow_request end + it 'expects a POST internal request with trailing slash to be allowed' do + expect(Rails.application.routes).not_to receive(:recognize_path) + response = request.post("/api/#{API::API.version}/internal/") + + expect(response).not_to be_redirect + expect(subject).not_to disallow_request + end + it 'expects a graphql request to be allowed' do response = request.post("/api/graphql") @@ -77,6 +85,13 @@ RSpec.shared_examples 'write access for a read-only GitLab instance' do expect(subject).not_to disallow_request end + it 'expects a graphql request with trailing slash to be allowed' do + response = request.post("/api/graphql/") + + expect(response).not_to be_redirect + expect(subject).not_to disallow_request + end + context 'relative URL is configured' do before do stub_config_setting(relative_url_root: '/gitlab') @@ -88,6 +103,13 @@ RSpec.shared_examples 'write access for a read-only GitLab instance' do expect(response).not_to be_redirect expect(subject).not_to disallow_request end + + it 'expects a graphql request with trailing slash to be allowed' do + response = request.post("/gitlab/api/graphql/") + + expect(response).not_to be_redirect + expect(subject).not_to disallow_request + end end context 'sidekiq admin requests' do @@ -119,6 +141,19 @@ RSpec.shared_examples 'write access for a read-only GitLab instance' do expect(response).not_to be_redirect expect(subject).not_to disallow_request end + + it 'allows requests with trailing slash' do + path = File.join(mounted_at, 'admin/sidekiq') + response = request.post("#{path}/") + + expect(response).not_to be_redirect + expect(subject).not_to disallow_request + + response = request.get("#{path}/") + + expect(response).not_to be_redirect + expect(subject).not_to disallow_request + end end end @@ -138,6 +173,14 @@ RSpec.shared_examples 'write access for a read-only GitLab instance' do expect(response).not_to be_redirect expect(subject).not_to disallow_request end + + it "expects a POST #{description} URL with trailing slash to be allowed" do + expect(Rails.application.routes).to receive(:recognize_path).and_call_original + response = request.post("#{path}/") + + expect(response).not_to be_redirect + expect(subject).not_to disallow_request + end end where(:description, :path) do @@ -153,11 +196,18 @@ RSpec.shared_examples 'write access for a read-only GitLab instance' do expect(response).to be_redirect expect(subject).to disallow_request end + + it "expects a POST #{description} URL with trailing slash not to be allowed" do + response = request.post("#{path}/") + + expect(response).to be_redirect + expect(subject).to disallow_request + end end end end - context 'json requests to a read-only GitLab instance' do + context 'JSON requests to a read-only GitLab instance' do let(:fake_app) { lambda { |env| [200, { 'Content-Type' => 'application/json' }, ['OK']] } } let(:content_json) { { 'CONTENT_TYPE' => 'application/json' } } diff --git a/spec/tasks/gitlab/x509/update_rake_spec.rb b/spec/tasks/gitlab/x509/update_rake_spec.rb index 93e97ab38a..b166e73935 100644 --- a/spec/tasks/gitlab/x509/update_rake_spec.rb +++ b/spec/tasks/gitlab/x509/update_rake_spec.rb @@ -8,12 +8,13 @@ RSpec.describe 'gitlab:x509 namespace rake task' do end describe 'update_signatures' do - subject { run_rake_task('gitlab:x509:update_signatures') } - - let(:project) { create :project, :repository, path: X509Helpers::User1.path } + let(:user) { create(:user, email: X509Helpers::User1.certificate_email) } + let(:project) { create(:project, :repository, path: X509Helpers::User1.path, creator: user) } let(:x509_signed_commit) { project.commit_by(oid: '189a6c924013fc3fe40d6f1ec1dc20214183bc97') } let(:x509_commit) { Gitlab::X509::Commit.new(x509_signed_commit).signature } + subject { run_rake_task('gitlab:x509:update_signatures') } + it 'changes from unverified to verified if the certificate store contains the root certificate' do x509_commit @@ -22,21 +23,14 @@ RSpec.describe 'gitlab:x509 namespace rake task' do store.add_cert(certificate) allow(OpenSSL::X509::Store).to receive(:new).and_return(store) - expect(x509_commit.verification_status).to eq('unverified') expect_any_instance_of(Gitlab::X509::Commit).to receive(:update_signature!).and_call_original - - subject - - x509_commit.reload - expect(x509_commit.verification_status).to eq('verified') + expect { subject }.to change { x509_commit.reload.verification_status }.from('unverified').to('verified') end it 'returns if no signature is available' do - expect_any_instance_of(Gitlab::X509::Commit) do |x509_commit| - expect(x509_commit).not_to receive(:update_signature!) + expect_any_instance_of(Gitlab::X509::Commit).not_to receive(:update_signature!) - subject - end + subject end end end diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb index 7ea2288fd4..6d04dce0a1 100644 --- a/spec/tooling/danger/changelog_spec.rb +++ b/spec/tooling/danger/changelog_spec.rb @@ -21,26 +21,20 @@ RSpec.describe Tooling::Danger::Changelog do describe '#required_reasons' do subject { changelog.required_reasons } - context "added files contain a migration" do - let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } - - it { is_expected.to include(:db_changes) } - end - context "removed files contains a feature flag" do let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } it { is_expected.to include(:feature_flag_removed) } end - context "added files do not contain a migration" do - let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) } + context "removed files do not contain a feature flag" do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } it { is_expected.to be_empty } end - context "removed files do not contain a feature flag" do - let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } + context "added files contain a migration" do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } it { is_expected.to be_empty } end @@ -49,26 +43,20 @@ RSpec.describe Tooling::Danger::Changelog do describe '#required?' do subject { changelog.required? } - context 'added files contain a migration' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } - - it { is_expected.to be_truthy } - end - context "removed files contains a feature flag" do let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } it { is_expected.to be_truthy } end - context 'added files do not contain a migration' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) } + context "removed files do not contain a feature flag" do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } it { is_expected.to be_falsey } end - context "removed files do not contain a feature flag" do - let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } + context 'added files contain a migration' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } it { is_expected.to be_falsey } end @@ -220,20 +208,6 @@ RSpec.describe Tooling::Danger::Changelog do end end - context 'with a new migration file' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } - - context "when title is not changed from sanitization", :aggregate_failures do - it_behaves_like 'changelog required text', :db_changes - end - - context "when title needs sanitization", :aggregate_failures do - let(:mr_title) { 'DRAFT: Fake Title' } - - it_behaves_like 'changelog required text', :db_changes - end - end - context 'with a removed feature flag file' do let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } @@ -255,20 +229,6 @@ RSpec.describe Tooling::Danger::Changelog do end end - context 'with a new migration file' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } - - context "when title is not changed from sanitization", :aggregate_failures do - it_behaves_like 'changelog required text', :db_changes - end - - context "when title needs sanitization", :aggregate_failures do - let(:mr_title) { 'DRAFT: Fake Title' } - - it_behaves_like 'changelog required text', :db_changes - end - end - context 'with a removed feature flag file' do let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } diff --git a/tooling/danger/changelog.rb b/tooling/danger/changelog.rb index 065c737050..5351accb22 100644 --- a/tooling/danger/changelog.rb +++ b/tooling/danger/changelog.rb @@ -34,7 +34,6 @@ module Tooling }.freeze REQUIRED_CHANGELOG_REASONS = { - db_changes: 'introduces a database migration', feature_flag_removed: 'removes a feature flag' }.freeze REQUIRED_CHANGELOG_MESSAGE = { @@ -50,7 +49,6 @@ module Tooling def required_reasons [].tap do |reasons| - reasons << :db_changes if project_helper.changes.added.has_category?(:migration) reasons << :feature_flag_removed if project_helper.changes.deleted.has_category?(:feature_flag) end end diff --git a/workhorse-vendor/github.com/mwitkow/grpc-proxy/LICENSE.txt b/workhorse-vendor/github.com/mwitkow/grpc-proxy/LICENSE.txt deleted file mode 100644 index cbfdef8c5a..0000000000 --- a/workhorse-vendor/github.com/mwitkow/grpc-proxy/LICENSE.txt +++ /dev/null @@ -1,174 +0,0 @@ - Apache License - Version 2.0, January 2004 - http://www.apache.org/licenses/ - - TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION - - 1. Definitions. - - "License" shall mean the terms and conditions for use, reproduction, - and distribution as defined by Sections 1 through 9 of this document. - - "Licensor" shall mean the copyright owner or entity authorized by - the copyright owner that is granting the License. - - "Legal Entity" shall mean the union of the acting entity and all - other entities that control, are controlled by, or are under common - control with that entity. For the purposes of this definition, - "control" means (i) the power, direct or indirect, to cause the - direction or management of such entity, whether by contract or - otherwise, or (ii) ownership of fifty percent (50%) or more of the - outstanding shares, or (iii) beneficial ownership of such entity. - - "You" (or "Your") shall mean an individual or Legal Entity - exercising permissions granted by this License. - - "Source" form shall mean the preferred form for making modifications, - including but not limited to software source code, documentation - source, and configuration files. - - "Object" form shall mean any form resulting from mechanical - transformation or translation of a Source form, including but - not limited to compiled object code, generated documentation, - and conversions to other media types. - - "Work" shall mean the work of authorship, whether in Source or - Object form, made available under the License, as indicated by a - copyright notice that is included in or attached to the work - (an example is provided in the Appendix below). - - "Derivative Works" shall mean any work, whether in Source or Object - form, that is based on (or derived from) the Work and for which the - editorial revisions, annotations, elaborations, or other modifications - represent, as a whole, an original work of authorship. For the purposes - of this License, Derivative Works shall not include works that remain - separable from, or merely link (or bind by name) to the interfaces of, - the Work and Derivative Works thereof. - - "Contribution" shall mean any work of authorship, including - the original version of the Work and any modifications or additions - to that Work or Derivative Works thereof, that is intentionally - submitted to Licensor for inclusion in the Work by the copyright owner - or by an individual or Legal Entity authorized to submit on behalf of - the copyright owner. For the purposes of this definition, "submitted" - means any form of electronic, verbal, or written communication sent - to the Licensor or its representatives, including but not limited to - communication on electronic mailing lists, source code control systems, - and issue tracking systems that are managed by, or on behalf of, the - Licensor for the purpose of discussing and improving the Work, but - excluding communication that is conspicuously marked or otherwise - designated in writing by the copyright owner as "Not a Contribution." - - "Contributor" shall mean Licensor and any individual or Legal Entity - on behalf of whom a Contribution has been received by Licensor and - subsequently incorporated within the Work. - - 2. Grant of Copyright License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - copyright license to reproduce, prepare Derivative Works of, - publicly display, publicly perform, sublicense, and distribute the - Work and such Derivative Works in Source or Object form. - - 3. Grant of Patent License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - (except as stated in this section) patent license to make, have made, - use, offer to sell, sell, import, and otherwise transfer the Work, - where such license applies only to those patent claims licensable - by such Contributor that are necessarily infringed by their - Contribution(s) alone or by combination of their Contribution(s) - with the Work to which such Contribution(s) was submitted. If You - institute patent litigation against any entity (including a - cross-claim or counterclaim in a lawsuit) alleging that the Work - or a Contribution incorporated within the Work constitutes direct - or contributory patent infringement, then any patent licenses - granted to You under this License for that Work shall terminate - as of the date such litigation is filed. - - 4. Redistribution. You may reproduce and distribute copies of the - Work or Derivative Works thereof in any medium, with or without - modifications, and in Source or Object form, provided that You - meet the following conditions: - - (a) You must give any other recipients of the Work or - Derivative Works a copy of this License; and - - (b) You must cause any modified files to carry prominent notices - stating that You changed the files; and - - (c) You must retain, in the Source form of any Derivative Works - that You distribute, all copyright, patent, trademark, and - attribution notices from the Source form of the Work, - excluding those notices that do not pertain to any part of - the Derivative Works; and - - (d) If the Work includes a "NOTICE" text file as part of its - distribution, then any Derivative Works that You distribute must - include a readable copy of the attribution notices contained - within such NOTICE file, excluding those notices that do not - pertain to any part of the Derivative Works, in at least one - of the following places: within a NOTICE text file distributed - as part of the Derivative Works; within the Source form or - documentation, if provided along with the Derivative Works; or, - within a display generated by the Derivative Works, if and - wherever such third-party notices normally appear. The contents - of the NOTICE file are for informational purposes only and - do not modify the License. You may add Your own attribution - notices within Derivative Works that You distribute, alongside - or as an addendum to the NOTICE text from the Work, provided - that such additional attribution notices cannot be construed - as modifying the License. - - You may add Your own copyright statement to Your modifications and - may provide additional or different license terms and conditions - for use, reproduction, or distribution of Your modifications, or - for any such Derivative Works as a whole, provided Your use, - reproduction, and distribution of the Work otherwise complies with - the conditions stated in this License. - - 5. Submission of Contributions. Unless You explicitly state otherwise, - any Contribution intentionally submitted for inclusion in the Work - by You to the Licensor shall be under the terms and conditions of - this License, without any additional terms or conditions. - Notwithstanding the above, nothing herein shall supersede or modify - the terms of any separate license agreement you may have executed - with Licensor regarding such Contributions. - - 6. Trademarks. This License does not grant permission to use the trade - names, trademarks, service marks, or product names of the Licensor, - except as required for reasonable and customary use in describing the - origin of the Work and reproducing the content of the NOTICE file. - - 7. Disclaimer of Warranty. Unless required by applicable law or - agreed to in writing, Licensor provides the Work (and each - Contributor provides its Contributions) on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - implied, including, without limitation, any warranties or conditions - of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A - PARTICULAR PURPOSE. You are solely responsible for determining the - appropriateness of using or redistributing the Work and assume any - risks associated with Your exercise of permissions under this License. - - 8. Limitation of Liability. In no event and under no legal theory, - whether in tort (including negligence), contract, or otherwise, - unless required by applicable law (such as deliberate and grossly - negligent acts) or agreed to in writing, shall any Contributor be - liable to You for damages, including any direct, indirect, special, - incidental, or consequential damages of any character arising as a - result of this License or out of the use or inability to use the - Work (including but not limited to damages for loss of goodwill, - work stoppage, computer failure or malfunction, or any and all - other commercial damages or losses), even if such Contributor - has been advised of the possibility of such damages. - - 9. Accepting Warranty or Additional Liability. While redistributing - the Work or Derivative Works thereof, You may choose to offer, - and charge a fee for, acceptance of support, warranty, indemnity, - or other liability obligations and/or rights consistent with this - License. However, in accepting such obligations, You may act only - on Your own behalf and on Your sole responsibility, not on behalf - of any other Contributor, and only if You agree to indemnify, - defend, and hold each Contributor harmless for any liability - incurred by, or claims asserted against, such Contributor by reason - of your accepting any such warranty or additional liability. \ No newline at end of file diff --git a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/DOC.md b/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/DOC.md deleted file mode 100644 index 85c411a104..0000000000 --- a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/DOC.md +++ /dev/null @@ -1,83 +0,0 @@ -# proxy --- - import "github.com/mwitkow/grpc-proxy/proxy" - -Package proxy provides a reverse proxy handler for gRPC. - -The implementation allows a `grpc.Server` to pass a received ServerStream to a -ClientStream without understanding the semantics of the messages exchanged. It -basically provides a transparent reverse-proxy. - -This package is intentionally generic, exposing a `StreamDirector` function that -allows users of this package to implement whatever logic of backend-picking, -dialing and service verification to perform. - -See examples on documented functions. - -## Usage - -#### func Codec - -```go -func Codec() grpc.Codec -``` -Codec returns a proxying grpc.Codec with the default protobuf codec as parent. - -See CodecWithParent. - -#### func CodecWithParent - -```go -func CodecWithParent(fallback grpc.Codec) grpc.Codec -``` -CodecWithParent returns a proxying grpc.Codec with a user provided codec as -parent. - -This codec is *crucial* to the functioning of the proxy. It allows the proxy -server to be oblivious to the schema of the forwarded messages. It basically -treats a gRPC message frame as raw bytes. However, if the server handler, or the -client caller are not proxy-internal functions it will fall back to trying to -decode the message using a fallback codec. - -#### func RegisterService - -```go -func RegisterService(server *grpc.Server, director StreamDirector, serviceName string, methodNames ...string) -``` -RegisterService sets up a proxy handler for a particular gRPC service and -method. The behaviour is the same as if you were registering a handler method, -e.g. from a codegenerated pb.go file. - -This can *only* be used if the `server` also uses grpcproxy.CodecForServer() -ServerOption. - -#### func TransparentHandler - -```go -func TransparentHandler(director StreamDirector) grpc.StreamHandler -``` -TransparentHandler returns a handler that attempts to proxy all requests that -are not registered in the server. The indented use here is as a transparent -proxy, where the server doesn't know about the services implemented by the -backends. It should be used as a `grpc.UnknownServiceHandler`. - -This can *only* be used if the `server` also uses grpcproxy.CodecForServer() -ServerOption. - -#### type StreamDirector - -```go -type StreamDirector func(ctx context.Context, fullMethodName string) (*grpc.ClientConn, error) -``` - -StreamDirector returns a gRPC ClientConn to be used to forward the call to. - -The presence of the `Context` allows for rich filtering, e.g. based on Metadata -(headers). If no handling is meant to be done, a `codes.NotImplemented` gRPC -error should be returned. - -It is worth noting that the StreamDirector will be fired *after* all server-side -stream interceptors are invoked. So decisions around authorization, monitoring -etc. are better to be handled there. - -See the rather rich example. diff --git a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/README.md b/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/README.md deleted file mode 100644 index 85c411a104..0000000000 --- a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/README.md +++ /dev/null @@ -1,83 +0,0 @@ -# proxy --- - import "github.com/mwitkow/grpc-proxy/proxy" - -Package proxy provides a reverse proxy handler for gRPC. - -The implementation allows a `grpc.Server` to pass a received ServerStream to a -ClientStream without understanding the semantics of the messages exchanged. It -basically provides a transparent reverse-proxy. - -This package is intentionally generic, exposing a `StreamDirector` function that -allows users of this package to implement whatever logic of backend-picking, -dialing and service verification to perform. - -See examples on documented functions. - -## Usage - -#### func Codec - -```go -func Codec() grpc.Codec -``` -Codec returns a proxying grpc.Codec with the default protobuf codec as parent. - -See CodecWithParent. - -#### func CodecWithParent - -```go -func CodecWithParent(fallback grpc.Codec) grpc.Codec -``` -CodecWithParent returns a proxying grpc.Codec with a user provided codec as -parent. - -This codec is *crucial* to the functioning of the proxy. It allows the proxy -server to be oblivious to the schema of the forwarded messages. It basically -treats a gRPC message frame as raw bytes. However, if the server handler, or the -client caller are not proxy-internal functions it will fall back to trying to -decode the message using a fallback codec. - -#### func RegisterService - -```go -func RegisterService(server *grpc.Server, director StreamDirector, serviceName string, methodNames ...string) -``` -RegisterService sets up a proxy handler for a particular gRPC service and -method. The behaviour is the same as if you were registering a handler method, -e.g. from a codegenerated pb.go file. - -This can *only* be used if the `server` also uses grpcproxy.CodecForServer() -ServerOption. - -#### func TransparentHandler - -```go -func TransparentHandler(director StreamDirector) grpc.StreamHandler -``` -TransparentHandler returns a handler that attempts to proxy all requests that -are not registered in the server. The indented use here is as a transparent -proxy, where the server doesn't know about the services implemented by the -backends. It should be used as a `grpc.UnknownServiceHandler`. - -This can *only* be used if the `server` also uses grpcproxy.CodecForServer() -ServerOption. - -#### type StreamDirector - -```go -type StreamDirector func(ctx context.Context, fullMethodName string) (*grpc.ClientConn, error) -``` - -StreamDirector returns a gRPC ClientConn to be used to forward the call to. - -The presence of the `Context` allows for rich filtering, e.g. based on Metadata -(headers). If no handling is meant to be done, a `codes.NotImplemented` gRPC -error should be returned. - -It is worth noting that the StreamDirector will be fired *after* all server-side -stream interceptors are invoked. So decisions around authorization, monitoring -etc. are better to be handled there. - -See the rather rich example. diff --git a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/codec.go b/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/codec.go deleted file mode 100644 index 846b9c4ebc..0000000000 --- a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/codec.go +++ /dev/null @@ -1,70 +0,0 @@ -package proxy - -import ( - "fmt" - - "github.com/golang/protobuf/proto" - "google.golang.org/grpc" -) - -// Codec returns a proxying grpc.Codec with the default protobuf codec as parent. -// -// See CodecWithParent. -func Codec() grpc.Codec { - return CodecWithParent(&protoCodec{}) -} - -// CodecWithParent returns a proxying grpc.Codec with a user provided codec as parent. -// -// This codec is *crucial* to the functioning of the proxy. It allows the proxy server to be oblivious -// to the schema of the forwarded messages. It basically treats a gRPC message frame as raw bytes. -// However, if the server handler, or the client caller are not proxy-internal functions it will fall back -// to trying to decode the message using a fallback codec. -func CodecWithParent(fallback grpc.Codec) grpc.Codec { - return &rawCodec{fallback} -} - -type rawCodec struct { - parentCodec grpc.Codec -} - -type frame struct { - payload []byte -} - -func (c *rawCodec) Marshal(v interface{}) ([]byte, error) { - out, ok := v.(*frame) - if !ok { - return c.parentCodec.Marshal(v) - } - return out.payload, nil - -} - -func (c *rawCodec) Unmarshal(data []byte, v interface{}) error { - dst, ok := v.(*frame) - if !ok { - return c.parentCodec.Unmarshal(data, v) - } - dst.payload = data - return nil -} - -func (c *rawCodec) String() string { - return fmt.Sprintf("proxy>%s", c.parentCodec.String()) -} - -// protoCodec is a Codec implementation with protobuf. It is the default rawCodec for gRPC. -type protoCodec struct{} - -func (protoCodec) Marshal(v interface{}) ([]byte, error) { - return proto.Marshal(v.(proto.Message)) -} - -func (protoCodec) Unmarshal(data []byte, v interface{}) error { - return proto.Unmarshal(data, v.(proto.Message)) -} - -func (protoCodec) String() string { - return "proto" -} diff --git a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/director.go b/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/director.go deleted file mode 100644 index 371ca60500..0000000000 --- a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/director.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2017 Michal Witkowski. All Rights Reserved. -// See LICENSE for licensing terms. - -package proxy - -import ( - "golang.org/x/net/context" - "google.golang.org/grpc" -) - -// StreamDirector returns a gRPC ClientConn to be used to forward the call to. -// -// The presence of the `Context` allows for rich filtering, e.g. based on Metadata (headers). -// If no handling is meant to be done, a `codes.NotImplemented` gRPC error should be returned. -// -// The context returned from this function should be the context for the *outgoing* (to backend) call. In case you want -// to forward any Metadata between the inbound request and outbound requests, you should do it manually. However, you -// *must* propagate the cancel function (`context.WithCancel`) of the inbound context to the one returned. -// -// It is worth noting that the StreamDirector will be fired *after* all server-side stream interceptors -// are invoked. So decisions around authorization, monitoring etc. are better to be handled there. -// -// See the rather rich example. -type StreamDirector func(ctx context.Context, fullMethodName string) (context.Context, *grpc.ClientConn, error) diff --git a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/doc.go b/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/doc.go deleted file mode 100644 index 01328f332a..0000000000 --- a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/doc.go +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright 2017 Michal Witkowski. All Rights Reserved. -// See LICENSE for licensing terms. - -/* -Package proxy provides a reverse proxy handler for gRPC. - -The implementation allows a `grpc.Server` to pass a received ServerStream to a ClientStream without understanding -the semantics of the messages exchanged. It basically provides a transparent reverse-proxy. - -This package is intentionally generic, exposing a `StreamDirector` function that allows users of this package -to implement whatever logic of backend-picking, dialing and service verification to perform. - -See examples on documented functions. -*/ -package proxy diff --git a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/handler.go b/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/handler.go deleted file mode 100644 index 752f892a1f..0000000000 --- a/workhorse-vendor/github.com/mwitkow/grpc-proxy/proxy/handler.go +++ /dev/null @@ -1,162 +0,0 @@ -// Copyright 2017 Michal Witkowski. All Rights Reserved. -// See LICENSE for licensing terms. - -package proxy - -import ( - "io" - - "golang.org/x/net/context" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" -) - -var ( - clientStreamDescForProxying = &grpc.StreamDesc{ - ServerStreams: true, - ClientStreams: true, - } -) - -// RegisterService sets up a proxy handler for a particular gRPC service and method. -// The behaviour is the same as if you were registering a handler method, e.g. from a codegenerated pb.go file. -// -// This can *only* be used if the `server` also uses grpcproxy.CodecForServer() ServerOption. -func RegisterService(server *grpc.Server, director StreamDirector, serviceName string, methodNames ...string) { - streamer := &handler{director} - fakeDesc := &grpc.ServiceDesc{ - ServiceName: serviceName, - HandlerType: (*interface{})(nil), - } - for _, m := range methodNames { - streamDesc := grpc.StreamDesc{ - StreamName: m, - Handler: streamer.handler, - ServerStreams: true, - ClientStreams: true, - } - fakeDesc.Streams = append(fakeDesc.Streams, streamDesc) - } - server.RegisterService(fakeDesc, streamer) -} - -// TransparentHandler returns a handler that attempts to proxy all requests that are not registered in the server. -// The indented use here is as a transparent proxy, where the server doesn't know about the services implemented by the -// backends. It should be used as a `grpc.UnknownServiceHandler`. -// -// This can *only* be used if the `server` also uses grpcproxy.CodecForServer() ServerOption. -func TransparentHandler(director StreamDirector) grpc.StreamHandler { - streamer := &handler{director} - return streamer.handler -} - -type handler struct { - director StreamDirector -} - -// handler is where the real magic of proxying happens. -// It is invoked like any gRPC server stream and uses the gRPC server framing to get and receive bytes from the wire, -// forwarding it to a ClientStream established against the relevant ClientConn. -func (s *handler) handler(srv interface{}, serverStream grpc.ServerStream) error { - // little bit of gRPC internals never hurt anyone - fullMethodName, ok := grpc.MethodFromServerStream(serverStream) - if !ok { - return grpc.Errorf(codes.Internal, "lowLevelServerStream not exists in context") - } - // We require that the director's returned context inherits from the serverStream.Context(). - outgoingCtx, backendConn, err := s.director(serverStream.Context(), fullMethodName) - if err != nil { - return err - } - - clientCtx, clientCancel := context.WithCancel(outgoingCtx) - // TODO(mwitkow): Add a `forwarded` header to metadata, https://en.wikipedia.org/wiki/X-Forwarded-For. - clientStream, err := grpc.NewClientStream(clientCtx, clientStreamDescForProxying, backendConn, fullMethodName) - if err != nil { - return err - } - // Explicitly *do not close* s2cErrChan and c2sErrChan, otherwise the select below will not terminate. - // Channels do not have to be closed, it is just a control flow mechanism, see - // https://groups.google.com/forum/#!msg/golang-nuts/pZwdYRGxCIk/qpbHxRRPJdUJ - s2cErrChan := s.forwardServerToClient(serverStream, clientStream) - c2sErrChan := s.forwardClientToServer(clientStream, serverStream) - // We don't know which side is going to stop sending first, so we need a select between the two. - for i := 0; i < 2; i++ { - select { - case s2cErr := <-s2cErrChan: - if s2cErr == io.EOF { - // this is the happy case where the sender has encountered io.EOF, and won't be sending anymore./ - // the clientStream>serverStream may continue pumping though. - clientStream.CloseSend() - break - } else { - // however, we may have gotten a receive error (stream disconnected, a read error etc) in which case we need - // to cancel the clientStream to the backend, let all of its goroutines be freed up by the CancelFunc and - // exit with an error to the stack - clientCancel() - return grpc.Errorf(codes.Internal, "failed proxying s2c: %v", s2cErr) - } - case c2sErr := <-c2sErrChan: - // This happens when the clientStream has nothing else to offer (io.EOF), returned a gRPC error. In those two - // cases we may have received Trailers as part of the call. In case of other errors (stream closed) the trailers - // will be nil. - serverStream.SetTrailer(clientStream.Trailer()) - // c2sErr will contain RPC error from client code. If not io.EOF return the RPC error as server stream error. - if c2sErr != io.EOF { - return c2sErr - } - return nil - } - } - return grpc.Errorf(codes.Internal, "gRPC proxying should never reach this stage.") -} - -func (s *handler) forwardClientToServer(src grpc.ClientStream, dst grpc.ServerStream) chan error { - ret := make(chan error, 1) - go func() { - f := &frame{} - for i := 0; ; i++ { - if err := src.RecvMsg(f); err != nil { - ret <- err // this can be io.EOF which is happy case - break - } - if i == 0 { - // This is a bit of a hack, but client to server headers are only readable after first client msg is - // received but must be written to server stream before the first msg is flushed. - // This is the only place to do it nicely. - md, err := src.Header() - if err != nil { - ret <- err - break - } - if err := dst.SendHeader(md); err != nil { - ret <- err - break - } - } - if err := dst.SendMsg(f); err != nil { - ret <- err - break - } - } - }() - return ret -} - -func (s *handler) forwardServerToClient(src grpc.ServerStream, dst grpc.ClientStream) chan error { - ret := make(chan error, 1) - go func() { - f := &frame{} - for i := 0; ; i++ { - if err := src.RecvMsg(f); err != nil { - ret <- err // this can be io.EOF which is happy case - break - } - if err := dst.SendMsg(f); err != nil { - ret <- err - break - } - } - }() - return ret -} diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go index 445ca3a94c..8ab83a1d98 100644 --- a/workhorse/internal/api/api.go +++ b/workhorse/internal/api/api.go @@ -168,7 +168,10 @@ func singleJoiningSlash(a, b string) string { // joinURLPath is taken from reverseproxy.go:joinURLPath func joinURLPath(a *url.URL, b string) (path string, rawpath string) { - if a.RawPath == "" && b == "" { + // Avoid adding a trailing slash if the suffix is empty + if b == "" { + return a.Path, a.RawPath + } else if a.RawPath == "" { return singleJoiningSlash(a.Path, b), "" } diff --git a/workhorse/main_test.go b/workhorse/main_test.go index 4e169b5112..5729d2412b 100644 --- a/workhorse/main_test.go +++ b/workhorse/main_test.go @@ -536,7 +536,11 @@ func TestApiContentTypeBlock(t *testing.T) { func TestAPIFalsePositivesAreProxied(t *testing.T) { goodResponse := []byte(``) ts := testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) { - if r.Header.Get(secret.RequestHeader) != "" && r.Method != "GET" { + url := r.URL.String() + if url[len(url)-1] == '/' { + w.WriteHeader(500) + w.Write([]byte("PreAuthorize request included a trailing slash")) + } else if r.Header.Get(secret.RequestHeader) != "" && r.Method != "GET" { w.WriteHeader(500) w.Write([]byte("non-GET request went through PreAuthorize handler")) } else {