diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-19 04:45:44 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-19 04:45:44 +0300 |
commit | 85dc423f7090da0a52c73eb66faf22ddb20efff9 (patch) | |
tree | 9160f299afd8c80c038f08e1545be119f5e3f1e1 /app/controllers/concerns | |
parent | 15c2c8c66dbe422588e5411eee7e68f1fa440bb8 (diff) |
Add latest changes from gitlab-org/gitlab@13-4-stable-ee
Diffstat (limited to 'app/controllers/concerns')
-rw-r--r-- | app/controllers/concerns/authenticates_with_two_factor.rb | 79 | ||||
-rw-r--r-- | app/controllers/concerns/integrations_actions.rb | 11 | ||||
-rw-r--r-- | app/controllers/concerns/issuable_actions.rb | 2 | ||||
-rw-r--r-- | app/controllers/concerns/issuable_links.rb | 41 | ||||
-rw-r--r-- | app/controllers/concerns/notes_actions.rb | 2 | ||||
-rw-r--r-- | app/controllers/concerns/redis_tracking.rb | 47 | ||||
-rw-r--r-- | app/controllers/concerns/renders_notes.rb | 10 | ||||
-rw-r--r-- | app/controllers/concerns/send_file_upload.rb | 38 | ||||
-rw-r--r-- | app/controllers/concerns/snippets_actions.rb | 8 | ||||
-rw-r--r-- | app/controllers/concerns/wiki_actions.rb | 30 |
10 files changed, 203 insertions, 65 deletions
diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index b93c98a4790..9ff97f398f5 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -22,9 +22,15 @@ module AuthenticatesWithTwoFactor return handle_locked_user(user) unless user.can?(:log_in) session[:otp_user_id] = user.id - session[:user_updated_at] = user.updated_at + session[:user_password_hash] = Digest::SHA256.hexdigest(user.encrypted_password) + push_frontend_feature_flag(:webauthn) + + if user.two_factor_webauthn_enabled? + setup_webauthn_authentication(user) + else + setup_u2f_authentication(user) + end - setup_u2f_authentication(user) render 'devise/sessions/two_factor' end @@ -41,12 +47,16 @@ module AuthenticatesWithTwoFactor def authenticate_with_two_factor user = self.resource = find_user return handle_locked_user(user) unless user.can?(:log_in) - return handle_changed_user(user) if user_changed?(user) + return handle_changed_user(user) if user_password_changed?(user) if user_params[:otp_attempt].present? && session[:otp_user_id] authenticate_with_two_factor_via_otp(user) elsif user_params[:device_response].present? && session[:otp_user_id] - authenticate_with_two_factor_via_u2f(user) + if user.two_factor_webauthn_enabled? + authenticate_with_two_factor_via_webauthn(user) + else + authenticate_with_two_factor_via_u2f(user) + end elsif user && user.valid_password?(user_params[:password]) prompt_for_two_factor(user) end @@ -66,7 +76,7 @@ module AuthenticatesWithTwoFactor def clear_two_factor_attempt! session.delete(:otp_user_id) - session.delete(:user_updated_at) + session.delete(:user_password_hash) session.delete(:challenge) end @@ -89,16 +99,17 @@ module AuthenticatesWithTwoFactor # Authenticate using the response from a U2F (universal 2nd factor) device def authenticate_with_two_factor_via_u2f(user) if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge]) - # Remove any lingering user data from login - clear_two_factor_attempt! + handle_two_factor_success(user) + else + handle_two_factor_failure(user, 'U2F') + end + end - remember_me(user) if user_params[:remember_me] == '1' - sign_in(user, message: :two_factor_authenticated, event: :authentication) + def authenticate_with_two_factor_via_webauthn(user) + if Webauthn::AuthenticateService.new(user, user_params[:device_response], session[:challenge]).execute + handle_two_factor_success(user) else - user.increment_failed_attempts! - Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=U2F") - flash.now[:alert] = _('Authentication via U2F device failed.') - prompt_for_two_factor(user) + handle_two_factor_failure(user, 'WebAuthn') end end @@ -116,8 +127,38 @@ module AuthenticatesWithTwoFactor sign_requests: sign_requests }) end end + + def setup_webauthn_authentication(user) + if user.webauthn_registrations.present? + + webauthn_registration_ids = user.webauthn_registrations.pluck(:credential_xid) + + get_options = WebAuthn::Credential.options_for_get(allow: webauthn_registration_ids, + user_verification: 'discouraged', + extensions: { appid: WebAuthn.configuration.origin }) + + session[:credentialRequestOptions] = get_options + session[:challenge] = get_options.challenge + gon.push(webauthn: { options: get_options.to_json }) + end + end # rubocop: enable CodeReuse/ActiveRecord + def handle_two_factor_success(user) + # Remove any lingering user data from login + clear_two_factor_attempt! + + remember_me(user) if user_params[:remember_me] == '1' + sign_in(user, message: :two_factor_authenticated, event: :authentication) + end + + def handle_two_factor_failure(user, method) + user.increment_failed_attempts! + Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=#{method}") + flash.now[:alert] = _('Authentication via %{method} device failed.') % { method: method } + prompt_for_two_factor(user) + end + def handle_changed_user(user) clear_two_factor_attempt! @@ -126,13 +167,9 @@ module AuthenticatesWithTwoFactor # If user has been updated since we validated the password, # the password might have changed. - def user_changed?(user) - return false unless session[:user_updated_at] - - # See: https://gitlab.com/gitlab-org/gitlab/-/issues/244638 - # Rounding errors happen when the user is updated, as the Rails ActiveRecord - # object has higher precision than what is stored in the database, therefore - # using .to_i to force truncation to the timestamp - user.updated_at.to_i != session[:user_updated_at].to_i + def user_password_changed?(user) + return false unless session[:user_password_hash] + + Digest::SHA256.hexdigest(user.encrypted_password) != session[:user_password_hash] end end diff --git a/app/controllers/concerns/integrations_actions.rb b/app/controllers/concerns/integrations_actions.rb index 9a8e5d14123..6060dc729af 100644 --- a/app/controllers/concerns/integrations_actions.rb +++ b/app/controllers/concerns/integrations_actions.rb @@ -16,12 +16,11 @@ module IntegrationsActions def update saved = integration.update(service_params[:service]) - overwrite = Gitlab::Utils.to_boolean(params[:overwrite]) respond_to do |format| format.html do if saved - PropagateIntegrationWorker.perform_async(integration.id, overwrite) + PropagateIntegrationWorker.perform_async(integration.id, false) redirect_to scoped_edit_integration_path(integration), notice: success_message else render 'shared/integrations/edit' @@ -57,9 +56,11 @@ module IntegrationsActions end def success_message - message = integration.active? ? _('activated') : _('settings saved, but not activated') - - _('%{service_title} %{message}.') % { service_title: integration.title, message: message } + if integration.active? + s_('Integrations|%{integration} settings saved and active.') % { integration: integration.title } + else + s_('Integrations|%{integration} settings saved, but not active.') % { integration: integration.title } + end end def serialize_as_json diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index c4dbce00593..a1a2740cde2 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -9,7 +9,7 @@ module IssuableActions before_action :check_destroy_confirmation!, only: :destroy before_action :authorize_admin_issuable!, only: :bulk_update before_action only: :show do - push_frontend_feature_flag(:scoped_labels, default_enabled: true) + push_frontend_feature_flag(:scoped_labels, type: :licensed, default_enabled: true) end before_action do push_frontend_feature_flag(:not_issuable_queries, @project, default_enabled: true) diff --git a/app/controllers/concerns/issuable_links.rb b/app/controllers/concerns/issuable_links.rb new file mode 100644 index 00000000000..2bdb190f1d5 --- /dev/null +++ b/app/controllers/concerns/issuable_links.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module IssuableLinks + def index + render json: issuables + end + + def create + result = create_service.execute + + render json: { message: result[:message], issuables: issuables }, status: result[:http_status] + end + + def destroy + result = destroy_service.execute + + render json: { issuables: issuables }, status: result[:http_status] + end + + private + + def issuables + list_service.execute + end + + def list_service + raise NotImplementedError + end + + def create_params + params.permit(issuable_references: []) + end + + def create_service + raise NotImplementedError + end + + def destroy_service + raise NotImplementedError + end +end diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index f4fc7decb60..7a5b470f366 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -100,7 +100,7 @@ module NotesActions # the finder. Here, we select between returning all notes since then, or a # page's worth of notes. def gather_notes - if Feature.enabled?(:paginated_notes, project) + if Feature.enabled?(:paginated_notes, noteable.try(:resource_parent)) gather_some_notes else gather_all_notes diff --git a/app/controllers/concerns/redis_tracking.rb b/app/controllers/concerns/redis_tracking.rb new file mode 100644 index 00000000000..fa5eef981d1 --- /dev/null +++ b/app/controllers/concerns/redis_tracking.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +# Example: +# +# # In controller include module +# # Track event for index action +# +# include RedisTracking +# +# track_redis_hll_event :index, :show, name: 'i_analytics_dev_ops_score', feature: :my_feature +# +# if the feature flag is enabled by default you should use +# track_redis_hll_event :index, :show, name: 'i_analytics_dev_ops_score', feature: :my_feature, feature_default_enabled: true +module RedisTracking + extend ActiveSupport::Concern + + class_methods do + def track_redis_hll_event(*controller_actions, name:, feature:, feature_default_enabled: false) + after_action only: controller_actions, if: -> { request.format.html? && request.headers['DNT'] != '1' } do + track_unique_redis_hll_event(name, feature, feature_default_enabled) + end + end + end + + private + + def track_unique_redis_hll_event(event_name, feature, feature_default_enabled) + return unless metric_feature_enabled?(feature, feature_default_enabled) + return unless Gitlab::CurrentSettings.usage_ping_enabled? + return unless visitor_id + + Gitlab::UsageDataCounters::HLLRedisCounter.track_event(visitor_id, event_name) + end + + def metric_feature_enabled?(feature, default_enabled) + Feature.enabled?(feature, default_enabled: default_enabled) + end + + def visitor_id + return cookies[:visitor_id] if cookies[:visitor_id].present? + return unless current_user + + uuid = SecureRandom.uuid + cookies[:visitor_id] = { value: uuid, expires: 24.months } + uuid + end +end diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb index 18015b1de88..f8e3717acee 100644 --- a/app/controllers/concerns/renders_notes.rb +++ b/app/controllers/concerns/renders_notes.rb @@ -5,7 +5,6 @@ module RendersNotes def prepare_notes_for_rendering(notes, noteable = nil) preload_noteable_for_regular_notes(notes) preload_max_access_for_authors(notes, @project) - preload_first_time_contribution_for_authors(noteable, notes) preload_author_status(notes) Notes::RenderService.new(current_user).execute(notes) @@ -19,7 +18,8 @@ module RendersNotes return unless project user_ids = notes.map(&:author_id) - project.team.max_member_access_for_user_ids(user_ids) + access = project.team.max_member_access_for_user_ids(user_ids).select { |k, v| v == Gitlab::Access::NO_ACCESS }.keys + project.team.contribution_check_for_user_ids(access) end # rubocop: disable CodeReuse/ActiveRecord @@ -28,12 +28,6 @@ module RendersNotes end # rubocop: enable CodeReuse/ActiveRecord - def preload_first_time_contribution_for_authors(noteable, notes) - return unless noteable.is_a?(Issuable) && noteable.first_contribution? - - notes.each {|n| n.specialize_for_first_contribution!(noteable)} - end - # rubocop: disable CodeReuse/ActiveRecord def preload_author_status(notes) ActiveRecord::Associations::Preloader.new.preload(notes, { author: :status }) diff --git a/app/controllers/concerns/send_file_upload.rb b/app/controllers/concerns/send_file_upload.rb index 7cb19fc7e58..2f06cd84ee5 100644 --- a/app/controllers/concerns/send_file_upload.rb +++ b/app/controllers/concerns/send_file_upload.rb @@ -2,6 +2,8 @@ module SendFileUpload def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, proxy: false, disposition: 'attachment') + content_type = content_type_for(attachment) + if attachment response_disposition = ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: attachment) @@ -9,7 +11,7 @@ module SendFileUpload # Google Cloud Storage, so the metadata needs to be cleared on GCS for # this to work. However, this override works with AWS. redirect_params[:query] = { "response-content-disposition" => response_disposition, - "response-content-type" => guess_content_type(attachment) } + "response-content-type" => content_type } # By default, Rails will send uploads with an extension of .js with a # content-type of text/javascript, which will trigger Rails' # cross-origin JavaScript protection. @@ -20,7 +22,7 @@ module SendFileUpload if image_scaling_request?(file_upload) location = file_upload.file_storage? ? file_upload.path : file_upload.url - headers.store(*Gitlab::Workhorse.send_scaled_image(location, params[:width].to_i)) + headers.store(*Gitlab::Workhorse.send_scaled_image(location, params[:width].to_i, content_type)) head :ok elsif file_upload.file_storage? send_file file_upload.path, send_params @@ -32,6 +34,12 @@ module SendFileUpload end end + def content_type_for(attachment) + return '' unless attachment + + guess_content_type(attachment) + end + def guess_content_type(filename) types = MIME::Types.type_for(filename) @@ -45,15 +53,33 @@ module SendFileUpload private def image_scaling_request?(file_upload) - avatar_image_upload?(file_upload) && valid_image_scaling_width? && current_user && - Feature.enabled?(:dynamic_image_resizing, current_user) + avatar_safe_for_scaling?(file_upload) && + scaling_allowed_by_feature_flags?(file_upload) && + valid_image_scaling_width? end - def avatar_image_upload?(file_upload) - file_upload.try(:image?) && file_upload.try(:mounted_as)&.to_sym == :avatar + def avatar_safe_for_scaling?(file_upload) + file_upload.try(:image_safe_for_scaling?) && mounted_as_avatar?(file_upload) + end + + def mounted_as_avatar?(file_upload) + file_upload.try(:mounted_as)&.to_sym == :avatar end def valid_image_scaling_width? Avatarable::ALLOWED_IMAGE_SCALER_WIDTHS.include?(params[:width]&.to_i) end + + # We use two separate feature gates to allow image resizing. + # The first, `:dynamic_image_resizing_requester`, based on the content requester. + # Enabling it for the user would allow that user to send resizing requests for any avatar. + # The second, `:dynamic_image_resizing_owner`, based on the content owner. + # Enabling it for the user would allow anyone to send resizing requests against the mentioned user avatar only. + # This flag allows us to operate on trusted data only, more in https://gitlab.com/gitlab-org/gitlab/-/issues/241533. + # Because of this, you need to enable BOTH to serve resized image, + # as you would need at least one allowed requester and at least one allowed avatar. + def scaling_allowed_by_feature_flags?(file_upload) + Feature.enabled?(:dynamic_image_resizing_requester, current_user) && + Feature.enabled?(:dynamic_image_resizing_owner, file_upload.model) + end end diff --git a/app/controllers/concerns/snippets_actions.rb b/app/controllers/concerns/snippets_actions.rb index 5552fd663f7..4548595d968 100644 --- a/app/controllers/concerns/snippets_actions.rb +++ b/app/controllers/concerns/snippets_actions.rb @@ -14,8 +14,6 @@ module SnippetsActions skip_before_action :verify_authenticity_token, if: -> { action_name == 'show' && js_request? } - before_action :redirect_if_binary, only: [:edit, :update] - respond_to :html end @@ -134,10 +132,4 @@ module SnippetsActions recaptcha_check_with_fallback(errors.empty?) { render action } end - - def redirect_if_binary - return if Feature.enabled?(:snippets_binary_blob) - - redirect_to gitlab_snippet_path(snippet) if blob&.binary? - end end diff --git a/app/controllers/concerns/wiki_actions.rb b/app/controllers/concerns/wiki_actions.rb index 5b953fe37d6..5a5b634da40 100644 --- a/app/controllers/concerns/wiki_actions.rb +++ b/app/controllers/concerns/wiki_actions.rb @@ -93,9 +93,10 @@ module WikiActions def update return render('shared/wikis/empty') unless can?(current_user, :create_wiki, container) - @page = WikiPages::UpdateService.new(container: container, current_user: current_user, params: wiki_params).execute(page) + response = WikiPages::UpdateService.new(container: container, current_user: current_user, params: wiki_params).execute(page) + @page = response.payload[:page] - if page.valid? + if response.success? redirect_to( wiki_page_path(wiki, page), notice: _('Wiki was successfully updated.') @@ -103,7 +104,7 @@ module WikiActions else render 'shared/wikis/edit' end - rescue WikiPage::PageChangedError, WikiPage::PageRenameError, Gitlab::Git::Wiki::OperationError => e + rescue WikiPage::PageChangedError, WikiPage::PageRenameError => e @error = e render 'shared/wikis/edit' end @@ -120,13 +121,8 @@ module WikiActions notice: _('Wiki was successfully updated.') ) else - flash[:alert] = response.message render 'shared/wikis/edit' end - rescue Gitlab::Git::Wiki::OperationError => e - @page = build_page(wiki_params) - @error = e - render 'shared/wikis/edit' end # rubocop:enable Gitlab/ModuleWithInstanceVariables @@ -162,14 +158,18 @@ module WikiActions # rubocop:disable Gitlab/ModuleWithInstanceVariables def destroy - WikiPages::DestroyService.new(container: container, current_user: current_user).execute(page) + return render_404 unless page - redirect_to wiki_path(wiki), - status: :found, - notice: _("Page was successfully deleted") - rescue Gitlab::Git::Wiki::OperationError => e - @error = e - render 'shared/wikis/edit' + response = WikiPages::DestroyService.new(container: container, current_user: current_user).execute(page) + + if response.success? + redirect_to wiki_path(wiki), + status: :found, + notice: _("Page was successfully deleted") + else + @error = response + render 'shared/wikis/edit' + end end # rubocop:enable Gitlab/ModuleWithInstanceVariables |