diff options
author | Andreas Brandl <abrandl@gitlab.com> | 2019-04-05 17:28:34 +0300 |
---|---|---|
committer | Andreas Brandl <abrandl@gitlab.com> | 2019-04-05 17:28:34 +0300 |
commit | 0cf23a7f50d51d91066cafc3d55bb846f7af3489 (patch) | |
tree | aea2704199d2b5c8b9e2767ae42288637322743b /app | |
parent | 6d25cd06acaa2f0f2c1cb422b613997c67eafc35 (diff) | |
parent | 46b1b9c1d61c269588bd3cd4203420608ddd7f0b (diff) |
Merge branch 'revert-3962b00b' into 'master'
Revert "Merge branch 'if-57131-external_auth_to_ce' into 'master'"
See merge request gitlab-org/gitlab-ce!27051
Diffstat (limited to 'app')
22 files changed, 10 insertions, 356 deletions
diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 79558b8604f..792c618fd40 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -67,10 +67,6 @@ } } -.classification-label { - background-color: $red-500; -} - .toggle-wrapper { margin-top: 5px; } diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index b681949ab36..ab792cf7403 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -124,9 +124,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController end def visible_application_setting_attributes - [ - *::ApplicationSettingsHelper.visible_attributes, - *::ApplicationSettingsHelper.external_authorization_service_attributes, + ApplicationSettingsHelper.visible_attributes + [ :domain_blacklist_file, disabled_oauth_sign_in_sources: [], import_sources: [], diff --git a/app/controllers/concerns/project_unauthorized.rb b/app/controllers/concerns/project_unauthorized.rb index d42363b8b17..f59440dbc59 100644 --- a/app/controllers/concerns/project_unauthorized.rb +++ b/app/controllers/concerns/project_unauthorized.rb @@ -1,21 +1,10 @@ # frozen_string_literal: true module ProjectUnauthorized - def project_unauthorized_proc - lambda do |project| - if project - label = project.external_authorization_classification_label - rejection_reason = nil - - unless ::Gitlab::ExternalAuthorization.access_allowed?(current_user, label) - rejection_reason = ::Gitlab::ExternalAuthorization.rejection_reason(current_user, label) - rejection_reason ||= _('External authorization denied access to this project') - end + extend ActiveSupport::Concern - if rejection_reason - access_denied!(rejection_reason) - end - end - end + # EE would override this + def project_unauthorized_proc + # no-op end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 632be29df72..94258e0e90a 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -345,7 +345,6 @@ class ProjectsController < Projects::ApplicationController :container_registry_enabled, :default_branch, :description, - :external_authorization_classification_label, :import_url, :issues_tracker, :issues_tracker_id, diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 50b5edf7da4..e635f608237 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -119,39 +119,6 @@ module ApplicationSettingsHelper options_for_select(options, selected) end - def external_authorization_description - _("If enabled, access to projects will be validated on an external service"\ - " using their classification label.") - end - - def external_authorization_timeout_help_text - _("Time in seconds GitLab will wait for a response from the external "\ - "service. When the service does not respond in time, access will be "\ - "denied.") - end - - def external_authorization_url_help_text - _("When leaving the URL blank, classification labels can still be "\ - "specified without disabling cross project features or performing "\ - "external authorization checks.") - end - - def external_authorization_client_certificate_help_text - _("The X509 Certificate to use when mutual TLS is required to communicate "\ - "with the external authorization service. If left blank, the server "\ - "certificate is still validated when accessing over HTTPS.") - end - - def external_authorization_client_key_help_text - _("The private key to use when a client certificate is provided. This value "\ - "is encrypted at rest.") - end - - def external_authorization_client_pass_help_text - _("The passphrase required to decrypt the private key. This is optional "\ - "and the value is encrypted at rest.") - end - def visible_attributes [ :admin_notification_email, @@ -270,18 +237,6 @@ module ApplicationSettingsHelper ] end - def external_authorization_service_attributes - [ - :external_auth_client_cert, - :external_auth_client_key, - :external_auth_client_key_pass, - :external_authorization_service_default_label, - :external_authorization_service_enabled, - :external_authorization_service_timeout, - :external_authorization_service_url - ] - end - def expanded_by_default? Rails.env.test? end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 2ac90eb8d9f..009dd70c2c9 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -303,16 +303,6 @@ module ProjectsHelper @path.present? end - def external_classification_label_help_message - default_label = ::Gitlab::CurrentSettings.current_application_settings - .external_authorization_service_default_label - - s_( - "ExternalAuthorizationService|When no classification label is set the "\ - "default label `%{default_label}` will be used." - ) % { default_label: default_label } - end - private def get_project_nav_tabs(project, current_user) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index d28a12413bf..7ec8505b33a 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -213,40 +213,6 @@ class ApplicationSetting < ApplicationRecord validate :terms_exist, if: :enforce_terms? - validates :external_authorization_service_default_label, - presence: true, - if: :external_authorization_service_enabled - - validates :external_authorization_service_url, - url: true, allow_blank: true, - if: :external_authorization_service_enabled - - validates :external_authorization_service_timeout, - numericality: { greater_than: 0, less_than_or_equal_to: 10 }, - if: :external_authorization_service_enabled - - validates :external_auth_client_key, - presence: true, - if: -> (setting) { setting.external_auth_client_cert.present? } - - validates_with X509CertificateCredentialsValidator, - certificate: :external_auth_client_cert, - pkey: :external_auth_client_key, - pass: :external_auth_client_key_pass, - if: -> (setting) { setting.external_auth_client_cert.present? } - - attr_encrypted :external_auth_client_key, - mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, - algorithm: 'aes-256-gcm', - encode: true - - attr_encrypted :external_auth_client_key_pass, - mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, - algorithm: 'aes-256-gcm', - encode: true - before_validation :ensure_uuid! before_validation :strip_sentry_values diff --git a/app/models/issue.rb b/app/models/issue.rb index eb4c87e05d5..97c6dcc4745 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -230,13 +230,7 @@ class Issue < ApplicationRecord def visible_to_user?(user = nil) return false unless project && project.feature_available?(:issues, user) - return publicly_visible? unless user - - return false unless readable_by?(user) - - user.full_private_access? || - ::Gitlab::ExternalAuthorization.access_allowed?( - user, project.external_authorization_classification_label) + user ? readable_by?(user) : publicly_visible? end def check_for_spam? @@ -304,7 +298,7 @@ class Issue < ApplicationRecord # Returns `true` if this Issue is visible to everybody. def publicly_visible? - project.public? && !confidential? && !::Gitlab::ExternalAuthorization.enabled? + project.public? && !confidential? end def expire_etag_cache diff --git a/app/models/project.rb b/app/models/project.rb index 97e287d3fa2..e2869fc2ad5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2062,11 +2062,6 @@ class Project < ApplicationRecord fetch_branch_allows_collaboration(user, branch_name) end - def external_authorization_classification_label - super || ::Gitlab::CurrentSettings.current_application_settings - .external_authorization_service_default_label - end - def licensed_features [] end diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 5dd2279ef99..72de04203a6 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -22,13 +22,6 @@ class BasePolicy < DeclarativePolicy::Base Gitlab::CurrentSettings.current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC) end - condition(:external_authorization_enabled, scope: :global, score: 0) do - ::Gitlab::ExternalAuthorization.perform_check? - end - - rule { external_authorization_enabled & ~full_private_access }.policy do - prevent :read_cross_project - end - + # This is prevented in some cases in `gitlab-ee` rule { default }.enable :read_cross_project end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index ba38af9c529..26d7d6e84c4 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -89,15 +89,6 @@ class ProjectPolicy < BasePolicy ::Gitlab::CurrentSettings.current_application_settings.mirror_available end - with_scope :subject - condition(:classification_label_authorized, score: 32) do - ::Gitlab::ExternalAuthorization.access_allowed?( - @user, - @subject.external_authorization_classification_label, - @subject.full_path - ) - end - # We aren't checking `:read_issue` or `:read_merge_request` in this case # because it could be possible for a user to see an issuable-iid # (`:read_issue_iid` or `:read_merge_request_iid`) but then wouldn't be @@ -426,25 +417,6 @@ class ProjectPolicy < BasePolicy rule { ~can_have_multiple_clusters & has_clusters }.prevent :add_cluster - rule { ~can?(:read_cross_project) & ~classification_label_authorized }.policy do - # Preventing access here still allows the projects to be listed. Listing - # projects doesn't check the `:read_project` ability. But instead counts - # on the `project_authorizations` table. - # - # All other actions should explicitly check read project, which would - # trigger the `classification_label_authorized` condition. - # - # `:read_project_for_iids` is not prevented by this condition, as it is - # used for cross-project reference checks. - prevent :guest_access - prevent :public_access - prevent :public_user_access - prevent :reporter_access - prevent :developer_access - prevent :maintainer_access - prevent :owner_access - end - private def team_member? diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb index 7eeaf8aade1..9146eb96533 100644 --- a/app/services/application_settings/update_service.rb +++ b/app/services/application_settings/update_service.rb @@ -2,17 +2,9 @@ module ApplicationSettings class UpdateService < ApplicationSettings::BaseService - include ValidatesClassificationLabel - attr_reader :params, :application_setting def execute - validate_classification_label(application_setting, :external_authorization_service_default_label) - - if application_setting.errors.any? - return false - end - update_terms(@params.delete(:terms)) if params.key?(:performance_bar_allowed_group_path) diff --git a/app/services/concerns/validates_classification_label.rb b/app/services/concerns/validates_classification_label.rb deleted file mode 100644 index ebcf5c24ff8..00000000000 --- a/app/services/concerns/validates_classification_label.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -module ValidatesClassificationLabel - def validate_classification_label(record, attribute_name) - return unless ::Gitlab::ExternalAuthorization.enabled? - return unless classification_label_change?(record, attribute_name) - - new_label = params[attribute_name].presence - new_label ||= ::Gitlab::CurrentSettings.current_application_settings - .external_authorization_service_default_label - - unless ::Gitlab::ExternalAuthorization.access_allowed?(current_user, new_label) - reason = rejection_reason_for_label(new_label) - message = s_('ClassificationLabelUnavailable|is unavailable: %{reason}') % { reason: reason } - record.errors.add(attribute_name, message) - end - end - - def rejection_reason_for_label(label) - reason_from_service = ::Gitlab::ExternalAuthorization.rejection_reason(current_user, label).presence - reason_from_service || _("Access to '%{classification_label}' not allowed") % { classification_label: label } - end - - def classification_label_change?(record, attribute_name) - params.key?(attribute_name) || record.new_record? - end -end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 3723c5ef7d7..d03137b63b2 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -2,8 +2,6 @@ module Projects class CreateService < BaseService - include ValidatesClassificationLabel - def initialize(user, params) @current_user, @params = user, params.dup @skip_wiki = @params.delete(:skip_wiki) @@ -47,8 +45,6 @@ module Projects relations_block&.call(@project) yield(@project) if block_given? - validate_classification_label(@project, :external_authorization_classification_label) - # If the block added errors, don't try to save the project return @project if @project.errors.any? diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index bc36bb8659d..6856009b395 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -3,7 +3,6 @@ module Projects class UpdateService < BaseService include UpdateVisibilityLevel - include ValidatesClassificationLabel ValidationError = Class.new(StandardError) @@ -15,8 +14,6 @@ module Projects yield if block_given? - validate_classification_label(project, :external_authorization_classification_label) - # If the block added errors, don't try to save the project return update_failed! if project.errors.any? diff --git a/app/validators/x509_certificate_credentials_validator.rb b/app/validators/x509_certificate_credentials_validator.rb deleted file mode 100644 index d2f18e956c3..00000000000 --- a/app/validators/x509_certificate_credentials_validator.rb +++ /dev/null @@ -1,86 +0,0 @@ -# frozen_string_literal: true - -# X509CertificateCredentialsValidator -# -# Custom validator to check if certificate-attribute was signed using the -# private key stored in an attrebute. -# -# This can be used as an `ActiveModel::Validator` as follows: -# -# validates_with X509CertificateCredentialsValidator, -# certificate: :client_certificate, -# pkey: :decrypted_private_key, -# pass: :decrypted_passphrase -# -# -# Required attributes: -# - certificate: The name of the accessor that returns the certificate to check -# - pkey: The name of the accessor that returns the private key -# Optional: -# - pass: The name of the accessor that returns the passphrase to decrypt the -# private key -class X509CertificateCredentialsValidator < ActiveModel::Validator - def initialize(*args) - super - - # We can't validate if we don't have a private key or certificate attributes - # in which case this validator is useless. - if options[:pkey].nil? || options[:certificate].nil? - raise 'Provide at least `certificate` and `pkey` attribute names' - end - end - - def validate(record) - unless certificate = read_certificate(record) - record.errors.add(options[:certificate], _('is not a valid X509 certificate.')) - end - - unless private_key = read_private_key(record) - record.errors.add(options[:pkey], _('could not read private key, is the passphrase correct?')) - end - - return if private_key.nil? || certificate.nil? - - unless certificate.public_key.fingerprint == private_key.public_key.fingerprint - record.errors.add(options[:pkey], _('private key does not match certificate.')) - end - end - - private - - def read_private_key(record) - OpenSSL::PKey.read(pkey(record).to_s, pass(record).to_s) - rescue OpenSSL::PKey::PKeyError, ArgumentError - # When the primary key could not be read, an ArgumentError is raised. - # This hapens when the passed key is not valid or the passphrase is incorrect - nil - end - - def read_certificate(record) - OpenSSL::X509::Certificate.new(certificate(record).to_s) - rescue OpenSSL::X509::CertificateError - nil - end - - # rubocop:disable GitlabSecurity/PublicSend - # - # Allowing `#public_send` here because we don't want the validator to really - # care about the names of the attributes or where they come from. - # - # The credentials are mostly stored encrypted so we need to go through the - # accessors to get the values, `read_attribute` bypasses those. - def certificate(record) - record.public_send(options[:certificate]) - end - - def pkey(record) - record.public_send(options[:pkey]) - end - - def pass(record) - return unless options[:pass] - - record.public_send(options[:pass]) - end - # rubocop:enable GitlabSecurity/PublicSend -end diff --git a/app/views/admin/application_settings/_external_authorization_service_form.html.haml b/app/views/admin/application_settings/_external_authorization_service_form.html.haml deleted file mode 100644 index 01f6c7afe61..00000000000 --- a/app/views/admin/application_settings/_external_authorization_service_form.html.haml +++ /dev/null @@ -1,51 +0,0 @@ -%section.settings.as-external-auth.no-animate#js-external-auth-settings{ class: ('expanded' if expanded) } - .settings-header - %h4 - = _('External authentication') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' - %p - = _('External Classification Policy Authorization') - .settings-content - - = form_for @application_setting, url: admin_application_settings_path(anchor: 'js-external-auth-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) - - %fieldset - .form-group - .form-check - = f.check_box :external_authorization_service_enabled, class: 'form-check-input' - = f.label :external_authorization_service_enabled, class: 'form-check-label' do - = _('Enable classification control using an external service') - %span.form-text.text-muted - = external_authorization_description - = link_to icon('question-circle'), help_page_path('user/admin_area/settings/external_authorization') - .form-group - = f.label :external_authorization_service_url, _('Service URL'), class: 'label-bold' - = f.text_field :external_authorization_service_url, class: 'form-control' - %span.form-text.text-muted - = external_authorization_url_help_text - .form-group - = f.label :external_authorization_service_timeout, _('External authorization request timeout'), class: 'label-bold' - = f.number_field :external_authorization_service_timeout, class: 'form-control', min: 0.001, max: 10, step: 0.001 - %span.form-text.text-muted - = external_authorization_timeout_help_text - = f.label :external_auth_client_cert, _('Client authentication certificate'), class: 'label-bold' - = f.text_area :external_auth_client_cert, class: 'form-control' - %span.form-text.text-muted - = external_authorization_client_certificate_help_text - .form-group - = f.label :external_auth_client_key, _('Client authentication key'), class: 'label-bold' - = f.text_area :external_auth_client_key, class: 'form-control' - %span.form-text.text-muted - = external_authorization_client_key_help_text - .form-group - = f.label :external_auth_client_key_pass, _('Client authentication key password'), class: 'label-bold' - = f.password_field :external_auth_client_key_pass, class: 'form-control' - %span.form-text.text-muted - = external_authorization_client_pass_help_text - .form-group - = f.label :external_authorization_service_default_label, _('Default classification label'), class: 'label-bold' - = f.text_field :external_authorization_service_default_label, class: 'form-control' - - = f.submit 'Save changes', class: "btn btn-success" diff --git a/app/views/admin/application_settings/show.html.haml b/app/views/admin/application_settings/show.html.haml index 31f18ba0d56..fc9dd29b8ca 100644 --- a/app/views/admin/application_settings/show.html.haml +++ b/app/views/admin/application_settings/show.html.haml @@ -68,7 +68,7 @@ .settings-content = render 'terms' -= render 'admin/application_settings/external_authorization_service_form', expanded: expanded_by_default? += render_if_exists 'admin/application_settings/external_authorization_service_form', expanded: expanded_by_default? %section.settings.as-terminal.no-animate#js-terminal-settings{ class: ('expanded' if expanded_by_default?) } .settings-header diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 26a1f1e119c..1b2a4cd6780 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -7,7 +7,6 @@ .alert-wrapper = render "layouts/broadcast" = render "layouts/header/read_only_banner" - = render "layouts/nav/classification_level_banner" = yield :flash_message = render "shared/ping_consent" - unless @hide_breadcrumbs diff --git a/app/views/layouts/nav/_classification_level_banner.html.haml b/app/views/layouts/nav/_classification_level_banner.html.haml deleted file mode 100644 index cc4caf079b8..00000000000 --- a/app/views/layouts/nav/_classification_level_banner.html.haml +++ /dev/null @@ -1,5 +0,0 @@ -- if ::Gitlab::ExternalAuthorization.enabled? && @project - = content_for :header_content do - %span.badge.color-label.classification-label.has-tooltip{ title: s_('ExternalAuthorizationService|Classification label') } - = sprite_icon('lock-open', size: 8, css_class: 'inline') - = @project.external_authorization_classification_label diff --git a/app/views/projects/_classification_policy_settings.html.haml b/app/views/projects/_classification_policy_settings.html.haml deleted file mode 100644 index 57c7a718d53..00000000000 --- a/app/views/projects/_classification_policy_settings.html.haml +++ /dev/null @@ -1,8 +0,0 @@ -- if ::Gitlab::ExternalAuthorization.enabled? - .form-group - = f.label :external_authorization_classification_label, class: 'label-bold' do - = s_('ExternalAuthorizationService|Classification Label') - %span.light (optional) - = f.text_field :external_authorization_classification_label, class: "form-control" - %span.form-text.text-muted - = external_classification_label_help_message diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index abf2fb7dc57..98017bea0c9 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -32,7 +32,7 @@ %span.light (optional) = f.text_area :description, class: "form-control", rows: 3, maxlength: 250 - = render 'projects/classification_policy_settings', f: f + = render_if_exists 'projects/classification_policy_settings', f: f = render_if_exists 'shared/repository_size_limit_setting', form: f, type: :project |