From b6316689fdc2d142af85b17d511d39e50712b420 Mon Sep 17 00:00:00 2001 From: Imre Farkas Date: Sun, 28 Oct 2018 18:27:15 +0100 Subject: [PATCH] Smartcard authentication with basic X.509 cert This adds support for smartcard authentication with basic X.509 certificates. NGINX needs to be setup in a way to run the same server context on separate port requesting the client side certificate. Another limitation is that the email address must be a part of the Common-Name attribute. --- app/controllers/application_controller.rb | 4 +- app/helpers/auth_helper.rb | 17 ++ app/views/devise/shared/_signin_box.html.haml | 9 +- app/views/devise/shared/_tabs_ldap.html.haml | 9 +- config/gitlab.yml.example | 11 ++ config/initializers/1_settings.rb | 3 + config/routes.rb | 1 + db/schema.rb | 9 + doc/administration/auth/README.md | 1 + doc/administration/auth/smartcard.md | 108 +++++++++++ ee/app/controllers/smartcard_controller.rb | 49 +++++ ee/app/helpers/ee/auth_helper.rb | 18 ++ ee/app/models/ee/user.rb | 7 + ee/app/models/license.rb | 1 + ee/app/models/smartcard_identity.rb | 13 ++ ee/app/services/ee/users/build_service.rb | 20 ++ .../devise/sessions/_new_smartcard.html.haml | 6 + .../devise/shared/_tab_smartcard.html.haml | 3 + .../unreleased/726-smartcard_auth.yml | 5 + ee/config/routes/smartcard.rb | 3 + ...81028092114_create_smartcard_identities.rb | 15 ++ ...92115_add_index_to_smartcard_identities.rb | 17 ++ ee/lib/gitlab/auth/smartcard.rb | 13 ++ ee/lib/gitlab/auth/smartcard/certificate.rb | 99 ++++++++++ ee/spec/factories/smartcard_identities.rb | 10 + ee/spec/features/users/login_spec.rb | 32 ++++ ee/spec/helpers/ee/auth_helper_spec.rb | 31 ++++ .../gitlab/auth/smartcard/certificate_spec.rb | 175 ++++++++++++++++++ ee/spec/models/ee/user_spec.rb | 11 ++ ee/spec/requests/smartcard_controller_spec.rb | 103 +++++++++++ locale/gitlab.pot | 12 ++ .../application_controller_spec.rb | 8 +- spec/features/users/login_spec.rb | 24 +-- spec/helpers/auth_helper_spec.rb | 10 + spec/support/helpers/user_login_helper.rb | 26 +++ 35 files changed, 851 insertions(+), 32 deletions(-) create mode 100644 doc/administration/auth/smartcard.md create mode 100644 ee/app/controllers/smartcard_controller.rb create mode 100644 ee/app/models/smartcard_identity.rb create mode 100644 ee/app/views/devise/sessions/_new_smartcard.html.haml create mode 100644 ee/app/views/devise/shared/_tab_smartcard.html.haml create mode 100644 ee/changelogs/unreleased/726-smartcard_auth.yml create mode 100644 ee/config/routes/smartcard.rb create mode 100644 ee/db/migrate/20181028092114_create_smartcard_identities.rb create mode 100644 ee/db/migrate/20181028092115_add_index_to_smartcard_identities.rb create mode 100644 ee/lib/gitlab/auth/smartcard.rb create mode 100644 ee/lib/gitlab/auth/smartcard/certificate.rb create mode 100644 ee/spec/factories/smartcard_identities.rb create mode 100644 ee/spec/helpers/ee/auth_helper_spec.rb create mode 100644 ee/spec/lib/gitlab/auth/smartcard/certificate_spec.rb create mode 100644 ee/spec/requests/smartcard_controller_spec.rb create mode 100644 spec/support/helpers/user_login_helper.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 89df994c58443e..e18488e8823e99 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -189,11 +189,11 @@ def can?(object, action, subject = :global) Ability.allowed?(object, action, subject) end - def access_denied!(message = nil) + def access_denied!(message = nil, status = nil) # If we display a custom access denied message to the user, we don't want to # hide existence of the resource, rather tell them they cannot access it using # the provided message - status = message.present? ? :forbidden : :not_found + status ||= message.present? ? :forbidden : :not_found respond_to do |format| format.any { head status } diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index e21ca3b1188640..b038f6e612fcd4 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -24,6 +24,23 @@ def label_for_provider(name) Gitlab::Auth::OAuth::Provider.label_for(name) end + def form_based_provider_priority + ['crowd', /^ldap/, 'kerberos'] + end + + def form_based_provider_with_highest_priority + @form_based_provider_with_highest_priority ||= begin + form_based_provider_priority.each do |provider_regexp| + highest_priority = form_based_providers.find { |provider| provider.match?(provider_regexp) } + break highest_priority unless highest_priority.nil? + end + end + end + + def form_based_auth_provider_has_active_class?(provider) + form_based_provider_with_highest_priority == provider + end + def form_based_provider?(name) [LDAP_PROVIDER, 'crowd'].any? { |pattern| pattern === name.to_s } end diff --git a/app/views/devise/shared/_signin_box.html.haml b/app/views/devise/shared/_signin_box.html.haml index 9fcbf0524dbf47..ca72498cb61a8e 100644 --- a/app/views/devise/shared/_signin_box.html.haml +++ b/app/views/devise/shared/_signin_box.html.haml @@ -1,18 +1,21 @@ - if form_based_providers.any? - if crowd_enabled? - .login-box.tab-pane.active{ id: "crowd", role: 'tabpanel' } + .login-box.tab-pane{ id: "crowd", role: 'tabpanel', class: active_when(form_based_auth_provider_has_active_class?(:crowd)) } .login-body = render 'devise/sessions/new_crowd' - if kerberos_enabled? - .login-box.tab-pane{ id: "kerberos", role: 'tabpanel', class: (:active unless crowd_enabled? || ldap_enabled?) } + .login-box.tab-pane{ id: "kerberos", role: 'tabpanel', class: active_when(form_based_auth_provider_has_active_class?(:kerberos)) } .login-body = render 'devise/sessions/new_kerberos' - @ldap_servers.each_with_index do |server, i| - .login-box.tab-pane{ id: "#{server['provider_name']}", role: 'tabpanel', class: active_when(i.zero? && !crowd_enabled?) } + .login-box.tab-pane{ id: "#{server['provider_name']}", role: 'tabpanel', class: active_when(i.zero? && form_based_auth_provider_has_active_class?(:ldapmain)) } .login-body = render 'devise/sessions/new_ldap', server: server + + = render_if_exists 'devise/sessions/new_smartcard' + - if password_authentication_enabled_for_web? .login-box.tab-pane{ id: 'login-pane', role: 'tabpanel' } .login-body diff --git a/app/views/devise/shared/_tabs_ldap.html.haml b/app/views/devise/shared/_tabs_ldap.html.haml index b58034e81af0f0..9f7d726f0af3ed 100644 --- a/app/views/devise/shared/_tabs_ldap.html.haml +++ b/app/views/devise/shared/_tabs_ldap.html.haml @@ -1,13 +1,16 @@ %ul.nav-links.new-session-tabs.nav-tabs.nav{ class: ('custom-provider-tabs' if form_based_providers.any?) } - if crowd_enabled? %li.nav-item - = link_to "Crowd", "#crowd", class: 'nav-link active', 'data-toggle' => 'tab' + = link_to "Crowd", "#crowd", class: "nav-link #{active_when(form_based_auth_provider_has_active_class?(:crowd))}", 'data-toggle' => 'tab' - if kerberos_enabled? %li.nav-item - = link_to "Kerberos", "#kerberos", class: "nav-link #{active_when(!crowd_enabled? && !ldap_enabled?)}", 'data-toggle' => 'tab' + = link_to "Kerberos", "#kerberos", class: "nav-link #{active_when(form_based_auth_provider_has_active_class?(:kerberos))}", 'data-toggle' => 'tab' - @ldap_servers.each_with_index do |server, i| %li.nav-item - = link_to server['label'], "##{server['provider_name']}", class: "nav-link #{active_when(i.zero? && !crowd_enabled?)} qa-ldap-tab", 'data-toggle' => 'tab' + = link_to server['label'], "##{server['provider_name']}", class: "nav-link #{active_when(i.zero? && form_based_auth_provider_has_active_class?(:ldapmain))} qa-ldap-tab", 'data-toggle' => 'tab' + + = render_if_exists 'devise/shared/tab_smartcard' + - if password_authentication_enabled_for_web? %li.nav-item = link_to 'Standard', '#login-pane', class: 'nav-link qa-standard-tab', 'data-toggle' => 'tab' diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 31f4e494535b3e..6f8d0829852eb4 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -550,6 +550,17 @@ production: &base # host: # .... + ## Smartcard authentication settings + smartcard: + # Allow smartcard authentication + enabled: false + + # Path to a file containing a CA certificate + ca_file: '/etc/ssl/certs/CA.pem' + + # Port where the client side certificate is requested by the webserver (NGINX/Apache) + # client_certificate_required_port: 3444 + ## Kerberos settings kerberos: # Allow the HTTP Negotiate authentication method for Git clients diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 054c83e799086c..a27c68ca35b569 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -50,6 +50,9 @@ end end +Settings['smartcard'] ||= Settingslogic.new({}) +Settings.smartcard['enabled'] = false if Settings.smartcard['enabled'].nil? + Settings['omniauth'] ||= Settingslogic.new({}) Settings.omniauth['enabled'] = true if Settings.omniauth['enabled'].nil? Settings.omniauth['auto_sign_in_with_provider'] = false if Settings.omniauth['auto_sign_in_with_provider'].nil? diff --git a/config/routes.rb b/config/routes.rb index 356ce0f603d4c8..7f49cb570f16dd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -89,6 +89,7 @@ draw :operations draw :instance_statistics + draw :smartcard if ENV['GITLAB_ENABLE_CHAOS_ENDPOINTS'] get '/chaos/leakmem' => 'chaos#leakmem' diff --git a/db/schema.rb b/db/schema.rb index 809cf2d6f6e728..a756b4f3a9d8ca 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2581,6 +2581,14 @@ t.index ["team_id", "alias"], name: "index_slack_integrations_on_team_id_and_alias", unique: true, using: :btree end + create_table "smartcard_identities", id: :bigserial, force: :cascade do |t| + t.integer "user_id", null: false + t.string "subject", null: false + t.string "issuer", null: false + t.index ["subject", "issuer"], name: "index_smartcard_identities_on_subject_and_issuer", unique: true, using: :btree + t.index ["user_id"], name: "index_smartcard_identities_on_user_id", using: :btree + end + create_table "snippets", force: :cascade do |t| t.string "title" t.text "content" @@ -3293,6 +3301,7 @@ add_foreign_key "saml_providers", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "services", "projects", name: "fk_71cce407f9", on_delete: :cascade add_foreign_key "slack_integrations", "services", on_delete: :cascade + add_foreign_key "smartcard_identities", "users", on_delete: :cascade add_foreign_key "snippets", "projects", name: "fk_be41fd4bb7", on_delete: :cascade add_foreign_key "software_license_policies", "projects", on_delete: :cascade add_foreign_key "software_license_policies", "software_licenses", on_delete: :cascade diff --git a/doc/administration/auth/README.md b/doc/administration/auth/README.md index d61450db7d6e35..2f32210480866f 100644 --- a/doc/administration/auth/README.md +++ b/doc/administration/auth/README.md @@ -16,3 +16,4 @@ providers. - [SAML](../../integration/saml.md) Configure GitLab as a SAML 2.0 Service Provider - [Okta](okta.md) Configure GitLab to sign in using Okta - [Authentiq](authentiq.md): Enable the Authentiq OmniAuth provider for passwordless authentication +- [Smartcard](smartcard.md) Smartcard authentication diff --git a/doc/administration/auth/smartcard.md b/doc/administration/auth/smartcard.md new file mode 100644 index 00000000000000..2b0d0ebe92c16e --- /dev/null +++ b/doc/administration/auth/smartcard.md @@ -0,0 +1,108 @@ +# Smartcard authentication + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/726) in +[GitLab Premium](https://about.gitlab.com/pricing/) 11.5 as an experimental +feature. Smartcard authentication may change or be removed completely in future +releases. + +Smartcards with X.509 certificates can be used to authenticate with GitLab. + +## X.509 certificates + +To use a smartcard with an X.509 certificate to authenticate with GitLab, `CN` +and `emailAddress` must be defined in the certificate. For example: + +``` +Certificate: + Data: + Version: 1 (0x0) + Serial Number: 12856475246677808609 (0xb26b601ecdd555e1) + Signature Algorithm: sha256WithRSAEncryption + Issuer: O=Random Corp Ltd, CN=Random Corp + Validity + Not Before: Oct 30 12:00:00 2018 GMT + Not After : Oct 30 12:00:00 2019 GMT + Subject: CN=Gitlab User, emailAddress=gitlab-user@example.com +``` + +## Configure NGINX to request a client side certificate + +In NGINX configuration, an **additional** server context must be defined with +the same configuration except: + +- The additional NGINX server context must be configured to run on a different + port: + + ``` + listen *:3444 ssl; + ``` + +- The additional NGINX server context must be configured to require the client + side certificate: + + ``` + ssl_verify_depth 2; + ssl_client_certificate /etc/ssl/certs/CA.pem; + ssl_verify_client on; + ``` + +- The additional NGINX server context must be configured to forward the client + side certificate: + + ``` + proxy_set_header X-SSL-Client-Certificate $ssl_client_escaped_cert; + ``` + +For example, the following is an example server context in an NGINX +configuration file (eg. in `/etc/nginx/sites-available/gitlab-ssl`): + +``` +server { + listen *:3444 ssl; + + # certificate for configuring SSL + ssl_certificate /path/to/example.com.crt; + ssl_certificate_key /path/to/example.com.key; + + ssl_verify_depth 2; + # CA certificate for client side certificate verification + ssl_client_certificate /etc/ssl/certs/CA.pem; + ssl_verify_client on; + + location / { + proxy_set_header Host $http_host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + + proxy_set_header X-SSL-Client-Certificate $ssl_client_escaped_cert; + + proxy_read_timeout 300; + + proxy_pass http://gitlab-workhorse; + } +} +``` + +## Configure GitLab for smartcard authentication + +**For installations from source** + +1. Edit `config/gitlab.yml`: + + ```yaml + ## Smartcard authentication settings + smartcard: + # Allow smartcard authentication + enabled: true + + # Path to a file containing a CA certificate + ca_file: '/etc/ssl/certs/CA.pem' + + # Port where the client side certificate is requested by NGINX + client_certificate_required_port: 3444 + ``` + +1. Save the file and restart GitLab for the changes to take effect. diff --git a/ee/app/controllers/smartcard_controller.rb b/ee/app/controllers/smartcard_controller.rb new file mode 100644 index 00000000000000..ddc42bee3f1cbd --- /dev/null +++ b/ee/app/controllers/smartcard_controller.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +class SmartcardController < ApplicationController + skip_before_action :authenticate_user! + skip_before_action :verify_authenticity_token + + before_action :check_feature_availability + before_action :check_certificate_headers + + def auth + certificate = Gitlab::Auth::Smartcard::Certificate.new(CGI.unescape(certificate_header)) + + user = certificate.find_or_create_user + unless user + flash[:alert] = _('Failed to signing using smartcard authentication') + redirect_to new_user_session_path(port: Gitlab.config.gitlab.port) + + return + end + + log_audit_event(user, with: 'smartcard') + sign_in_and_redirect(user) + end + + protected + + def check_feature_availability + render_404 unless ::Gitlab::Auth::Smartcard.enabled? + end + + def check_certificate_headers + # Failing on requests coming from the port not requiring client side certificate + unless certificate_header.present? + access_denied!(_('Smartcard authentication failed: client certificate header is missing.'), 401) + end + end + + def log_audit_event(user, options = {}) + AuditEventService.new(user, user, options).for_authentication.security_event + end + + def certificate_header + request.headers['HTTP_X_SSL_CLIENT_CERTIFICATE'] + end + + def after_sign_in_path_for(resource) + stored_location_for(:redirect) || stored_location_for(resource) || root_url(port: Gitlab.config.gitlab.port) + end +end diff --git a/ee/app/helpers/ee/auth_helper.rb b/ee/app/helpers/ee/auth_helper.rb index 10436b37bf95de..c03cdd7f223e4e 100644 --- a/ee/app/helpers/ee/auth_helper.rb +++ b/ee/app/helpers/ee/auth_helper.rb @@ -16,15 +16,33 @@ def providers_for_base_controller super - GROUP_LEVEL_PROVIDERS end + override :form_based_provider_priority + def form_based_provider_priority + super << 'smartcard' + end + override :form_based_provider? def form_based_provider?(name) super || name.to_s == 'kerberos' end + override :form_based_providers + def form_based_providers + providers = super + + providers << :smartcard if smartcard_enabled? + + providers + end + def kerberos_enabled? auth_providers.include?(:kerberos) end + def smartcard_enabled? + ::Gitlab::Auth::Smartcard.enabled? + end + def slack_redirect_uri(project) slack_auth_project_settings_slack_url(project) end diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index fd0c5e865f3845..eed1c84bd427cc 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -45,6 +45,8 @@ module User has_many :protected_branch_push_access_levels, dependent: :destroy, class_name: ::ProtectedBranch::PushAccessLevel # rubocop:disable Cop/ActiveRecordDependent has_many :protected_branch_unprotect_access_levels, dependent: :destroy, class_name: ::ProtectedBranch::UnprotectAccessLevel # rubocop:disable Cop/ActiveRecordDependent + has_many :smartcard_identities + scope :excluding_guests, -> { joins(:members).where('members.access_level > ?', ::Gitlab::Access::GUEST).distinct } scope :subscribed_for_admin_email, -> { where(admin_email_unsubscribed_at: nil) } @@ -77,6 +79,11 @@ def non_ldap joins('LEFT JOIN identities ON identities.user_id = users.id') .where('identities.provider IS NULL OR identities.provider NOT LIKE ?', 'ldap%') end + + def find_by_smartcard_identity(certificate_subject, certificate_issuer) + joins(:smartcard_identities) + .find_by(smartcard_identities: { subject: certificate_subject, issuer: certificate_issuer }) + end end def cannot_be_admin_and_auditor diff --git a/ee/app/models/license.rb b/ee/app/models/license.rb index c265772c264e87..24707d6bfdcec2 100644 --- a/ee/app/models/license.rb +++ b/ee/app/models/license.rb @@ -57,6 +57,7 @@ class License < ActiveRecord::Base object_storage group_saml service_desk + smartcard_auth unprotection_restrictions variable_environment_scope reject_unsigned_commits diff --git a/ee/app/models/smartcard_identity.rb b/ee/app/models/smartcard_identity.rb new file mode 100644 index 00000000000000..b708d5d0a8e4cd --- /dev/null +++ b/ee/app/models/smartcard_identity.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class SmartcardIdentity < ActiveRecord::Base + belongs_to :user + + validates :user, + presence: true + validates :subject, + presence: true, + uniqueness: { scope: :issuer } + validates :issuer, + presence: true +end diff --git a/ee/app/services/ee/users/build_service.rb b/ee/app/services/ee/users/build_service.rb index 7607cf08e1b541..51faa5623b99e0 100644 --- a/ee/app/services/ee/users/build_service.rb +++ b/ee/app/services/ee/users/build_service.rb @@ -1,6 +1,17 @@ module EE module Users module BuildService + extend ::Gitlab::Utils::Override + + override :execute + def execute(skip_authorization: false) + user = super + + build_smartcard_identity(user, params) if ::Gitlab::Auth::Smartcard.enabled? + + user + end + private def signup_params @@ -15,6 +26,15 @@ def email_opted_in_params :email_opted_in_at ] end + + def build_smartcard_identity(user, params) + smartcard_identity_attrs = params.slice(:certificate_subject, :certificate_issuer) + + if smartcard_identity_attrs.any? + user.smartcard_identities.build(subject: params[:certificate_subject], + issuer: params[:certificate_issuer]) + end + end end end end diff --git a/ee/app/views/devise/sessions/_new_smartcard.html.haml b/ee/app/views/devise/sessions/_new_smartcard.html.haml new file mode 100644 index 00000000000000..510848305245a3 --- /dev/null +++ b/ee/app/views/devise/sessions/_new_smartcard.html.haml @@ -0,0 +1,6 @@ +- if smartcard_enabled? + .login-box.tab-pane{ id: 'smartcard', role: 'tabpanel', class: active_when(form_based_auth_provider_has_active_class?(:smartcard)) } + .login-body + = form_tag(smartcard_auth_url(port: Gitlab.config.smartcard.client_certificate_required_port), html: { 'aria-live' => 'assertive'}) do + .submit-container + = submit_tag _('Login with smartcard'), class: 'btn btn-success' diff --git a/ee/app/views/devise/shared/_tab_smartcard.html.haml b/ee/app/views/devise/shared/_tab_smartcard.html.haml new file mode 100644 index 00000000000000..e1ce7ef9387be4 --- /dev/null +++ b/ee/app/views/devise/shared/_tab_smartcard.html.haml @@ -0,0 +1,3 @@ +- if smartcard_enabled? + %li.nav-item + = link_to _('Smartcard'), '#smartcard', class: "nav-link #{active_when(form_based_auth_provider_has_active_class?(:smartcard))}", 'data-toggle' => 'tab' diff --git a/ee/changelogs/unreleased/726-smartcard_auth.yml b/ee/changelogs/unreleased/726-smartcard_auth.yml new file mode 100644 index 00000000000000..4005b8f0124ca6 --- /dev/null +++ b/ee/changelogs/unreleased/726-smartcard_auth.yml @@ -0,0 +1,5 @@ +--- +title: Smartcard authentication +merge_request: 8120 +author: +type: added diff --git a/ee/config/routes/smartcard.rb b/ee/config/routes/smartcard.rb new file mode 100644 index 00000000000000..4141e1ed2d62b3 --- /dev/null +++ b/ee/config/routes/smartcard.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +post 'smartcard/auth' => 'smartcard#auth' diff --git a/ee/db/migrate/20181028092114_create_smartcard_identities.rb b/ee/db/migrate/20181028092114_create_smartcard_identities.rb new file mode 100644 index 00000000000000..a1efcecfacc19f --- /dev/null +++ b/ee/db/migrate/20181028092114_create_smartcard_identities.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateSmartcardIdentities < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :smartcard_identities, id: :bigserial do |t| + t.references :user, null: false, index: true, foreign_key: { on_delete: :cascade } + t.string 'subject', null: false + t.string 'issuer', null: false + end + end +end diff --git a/ee/db/migrate/20181028092115_add_index_to_smartcard_identities.rb b/ee/db/migrate/20181028092115_add_index_to_smartcard_identities.rb new file mode 100644 index 00000000000000..db6cc165250952 --- /dev/null +++ b/ee/db/migrate/20181028092115_add_index_to_smartcard_identities.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToSmartcardIdentities < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :smartcard_identities, [:subject, :issuer], unique: true + end + + def down + remove_concurrent_index :smartcard_identities, [:subject, :issuer] + end +end diff --git a/ee/lib/gitlab/auth/smartcard.rb b/ee/lib/gitlab/auth/smartcard.rb new file mode 100644 index 00000000000000..8c04cbba701da9 --- /dev/null +++ b/ee/lib/gitlab/auth/smartcard.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Gitlab + module Auth + module Smartcard + extend self + + def enabled? + ::License.feature_available?(:smartcard_auth) && ::Gitlab.config.smartcard.enabled + end + end + end +end diff --git a/ee/lib/gitlab/auth/smartcard/certificate.rb b/ee/lib/gitlab/auth/smartcard/certificate.rb new file mode 100644 index 00000000000000..b779941dd34798 --- /dev/null +++ b/ee/lib/gitlab/auth/smartcard/certificate.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module Gitlab + module Auth + module Smartcard + class Certificate + InvalidCAFilePath = Class.new(StandardError) + InvalidCertificate = Class.new(StandardError) + + delegate :allow_signup?, + to: :'Gitlab::CurrentSettings.current_application_settings' + + def self.store + @store ||= OpenSSL::X509::Store.new.tap do |store| + store.add_cert( + OpenSSL::X509::Certificate.new( + File.read(Gitlab.config.smartcard.ca_file))) + end + rescue Errno::ENOENT => ex + Gitlab::AppLogger.error('Failed to open Gitlab.config.smartcard.ca_file') + Gitlab::AppLogger.error(ex) + raise InvalidCAFilePath + rescue OpenSSL::X509::CertificateError => ex + Gitlab::AppLogger.error('Gitlab.config.smartcard.ca_file is not a valid certificate') + Gitlab::AppLogger.error(ex) + raise InvalidCertificate + end + + def initialize(certificate) + @certificate = OpenSSL::X509::Certificate.new(certificate) + @subject = @certificate.subject.to_s + @issuer = @certificate.issuer.to_s + rescue OpenSSL::X509::CertificateError + # no-op + end + + def find_or_create_user + return unless valid? + + user = find_user + user ||= create_user if allow_signup? + user + end + + private + + def valid? + self.class.store.verify(@certificate) if @certificate + end + + def find_user + User.find_by_smartcard_identity(@subject, @issuer) + end + + def create_user + user = User.find_by_email(email) + if user + create_smartcard_identity_for(user) + return user + end + + user_params = { + name: common_name, + username: username, + email: email, + password: password, + password_confirmation: password, + password_automatically_set: true, + certificate_subject: @subject, + certificate_issuer: @issuer, + skip_confirmation: true + } + + Users::CreateService.new(nil, user_params).execute(skip_authorization: true) + end + + def create_smartcard_identity_for(user) + SmartcardIdentity.create(user: user, subject: @subject, issuer: @issuer) + end + + def common_name + @subject.split('/').find { |part| part =~ /CN=/ }&.remove('CN=')&.strip + end + + def email + @subject.split('/').find { |part| part =~ /emailAddress=/ }&.remove('emailAddress=')&.strip + end + + def username + ::Namespace.clean_path(common_name) + end + + def password + @password ||= Devise.friendly_token(8) + end + end + end + end +end diff --git a/ee/spec/factories/smartcard_identities.rb b/ee/spec/factories/smartcard_identities.rb new file mode 100644 index 00000000000000..5c7b8953991c03 --- /dev/null +++ b/ee/spec/factories/smartcard_identities.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :smartcard_identity do + subject { 'CN=gitlab-user/emailAddress=gitlab-user@random-corp.org' } + issuer { 'O=Random Corp Ltd, CN=Random Corp' } + + association :user + end +end diff --git a/ee/spec/features/users/login_spec.rb b/ee/spec/features/users/login_spec.rb index b73f662433b7ed..6a8f842bd8110e 100644 --- a/ee/spec/features/users/login_spec.rb +++ b/ee/spec/features/users/login_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe 'Login' do + include UserLoginHelper + before do stub_licensed_features(extended_audit_events: true) end @@ -25,4 +27,34 @@ expect { gitlab_sign_in_via('saml', user, 'wrong-uid') } .to change { SecurityEvent.where(entity_id: -1).count }.from(0).to(1) end + + describe 'UI tabs and panes' do + context 'when smartcard is enabled' do + before do + visit new_user_session_path + allow(page).to receive(:form_based_providers).and_return([:smartcard]) + allow(page).to receive(:smartcard_enabled?).and_return(true) + end + + context 'with smartcard_auth feature flag off' do + before do + stub_licensed_features(smartcard_auth: false) + end + + it 'correctly renders tabs and panes' do + ensure_tab_pane_correctness(false) + end + end + + context 'with smartcard_auth feature flag on' do + before do + stub_licensed_features(smartcard_auth: true) + end + + it 'correctly renders tabs and panes' do + ensure_tab_pane_correctness(false) + end + end + end + end end diff --git a/ee/spec/helpers/ee/auth_helper_spec.rb b/ee/spec/helpers/ee/auth_helper_spec.rb new file mode 100644 index 00000000000000..3826175e6f9e42 --- /dev/null +++ b/ee/spec/helpers/ee/auth_helper_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true +# +require 'spec_helper' + +describe EE::AuthHelper do + describe "form_based_providers" do + context 'with smartcard_auth feature flag off' do + before do + stub_licensed_features(smartcard_auth: false) + allow(helper).to receive(:smartcard_enabled?).and_call_original + end + + it 'does not include smartcard provider' do + allow(helper).to receive(:auth_providers) { [:twitter, :smartcard] } + expect(helper.form_based_providers).to be_empty + end + end + + context 'with smartcard_auth feature flag on' do + before do + stub_licensed_features(smartcard_auth: true) + allow(helper).to receive(:smartcard_enabled?).and_return(true) + end + + it 'includes smartcard provider' do + allow(helper).to receive(:auth_providers) { [:twitter, :smartcard] } + expect(helper.form_based_providers).to eq %i(smartcard) + end + end + end +end diff --git a/ee/spec/lib/gitlab/auth/smartcard/certificate_spec.rb b/ee/spec/lib/gitlab/auth/smartcard/certificate_spec.rb new file mode 100644 index 00000000000000..482e8d852a5155 --- /dev/null +++ b/ee/spec/lib/gitlab/auth/smartcard/certificate_spec.rb @@ -0,0 +1,175 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Auth::Smartcard::Certificate do + let(:subject_dn) { '/O=Random Corp Ltd/CN=gitlab-user/emailAddress=gitlab-user@random-corp.org' } + let(:issuer_dn) { '/O=Random Corp Ltd/CN=Random Corp' } + let(:certificate_header) { 'certificate' } + let(:openssl_certificate_store) { instance_double(OpenSSL::X509::Store) } + let(:openssl_certificate) { instance_double(OpenSSL::X509::Certificate, subject: subject_dn, issuer: issuer_dn) } + let(:user_build_service) { instance_double(Users::BuildService) } + + before do + allow(described_class).to receive(:store).and_return(openssl_certificate_store) + allow(OpenSSL::X509::Certificate).to receive(:new).and_return(openssl_certificate) + allow(openssl_certificate_store).to receive(:verify).and_return(true) + end + + shared_examples 'a new smartcard identity' do + it 'creates smartcard identity' do + expect { subject }.to change { SmartcardIdentity.count }.from(0).to(1) + + identity = SmartcardIdentity.first + expect(identity.subject).to eql(subject_dn) + expect(identity.issuer).to eql(issuer_dn) + end + end + + shared_examples 'an existing user' do + it 'finds existing user' do + expect(subject).to eql(user) + end + + it 'does not create new user' do + expect { subject }.not_to change { User.count } + end + end + + describe '#find_or_create_user' do + subject { described_class.new(certificate_header).find_or_create_user } + + context 'user and smartcard identity already exist' do + let(:user) { create(:user) } + + before do + create(:smartcard_identity, subject: subject_dn, issuer: issuer_dn, user: user) + end + + it_behaves_like 'an existing user' + end + + context 'user exists but smartcard identity does not' do + let!(:user) { create(:user, email: 'gitlab-user@random-corp.org') } + + it_behaves_like 'an existing user' + + it_behaves_like 'a new smartcard identity' + + it 'associates the new smartcard identity with the user' do + subject + + expect(SmartcardIdentity.first.user).to eql(user) + end + end + + context 'user and smartcard identity do not exist' do + let(:user) { create(:user) } + + before do + allow(Gitlab::Auth::Smartcard).to receive(:enabled?).and_return(true) + end + + it 'creates user' do + expect { subject }.to change { User.count }.from(0).to(1) + expect(User.first.username).to eql('gitlab-user') + expect(User.first.email).to eql('gitlab-user@random-corp.org') + end + + it_behaves_like 'a new smartcard identity' + + it 'calls Users::BuildService with correct params' do + user_params = { name: 'gitlab-user', + username: 'gitlab-user', + email: 'gitlab-user@random-corp.org', + password_automatically_set: true, + certificate_subject: subject_dn, + certificate_issuer: issuer_dn, + skip_confirmation: true } + expect(Users::BuildService).to( + receive(:new) + .with(nil, hash_including(user_params)) + .and_return(user_build_service)) + expect(user_build_service).to( + receive(:execute).with(skip_authorization: true).and_return(user)) + + subject + end + + context 'username generation' do + context 'uses CN from certificate' do + let(:subject_dn) { '/CN=Gitlab User/emailAddress=gitlab-user@random-corp.org' } + + it 'creates user with correct username' do + subject + + expect(User.first.username).to eql('GitlabUser') + end + end + + context 'avoids conflicting namespaces' do + let(:subject_dn) { '/CN=Gitlab User/emailAddress=gitlab-user@random-corp.org' } + let!(:existing_user) { create(:user, username: 'GitlabUser') } + + it 'creates user with correct usnername' do + expect { subject }.to change { User.count }.from(1).to(2) + expect(User.last.username).to eql('GitlabUser1') + end + end + end + end + + context 'invalid certificate' do + before do + allow(openssl_certificate_store).to receive(:verify).and_return(false) + end + + it 'returns nil' do + expect(subject).to be_nil + end + end + + context 'incorrect certificate' do + before do + allow(OpenSSL::X509::Certificate).to receive(:new).and_call_original + end + + it 'returns nil' do + expect(subject).to be_nil + end + end + end + + describe '.store' do + before do + allow(Gitlab.config.smartcard).to receive(:ca_file).and_return('ca_file') + allow(described_class).to receive(:store).and_call_original + allow(OpenSSL::X509::Certificate).to receive(:new).and_call_original + clear_store + end + after do + clear_store + end + + subject { described_class.store } + + context 'file does not exist' do + it 'raises error' do + expect { subject }.to raise_error(Gitlab::Auth::Smartcard::Certificate::InvalidCAFilePath) + end + end + + context 'smartcard.ca_file is not a valid certificate' do + it 'raises error' do + expect(File).to receive(:read).with('ca_file').and_return('invalid certificate') + expect { subject }.to raise_error(Gitlab::Auth::Smartcard::Certificate::InvalidCertificate) + end + end + end + + def clear_store + described_class.remove_instance_variable(:@store) + rescue NameError + # raised if @store was not set; ignore + end +end diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index baff99a23ea82f..86865058684cb7 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -28,6 +28,17 @@ end end + describe '.find_by_smartcard_identity' do + let!(:user) { create(:user) } + let!(:smartcard_identity) { create(:smartcard_identity, user: user) } + + it 'returns the user' do + expect(User.find_by_smartcard_identity(smartcard_identity.subject, + smartcard_identity.issuer)) + .to eq(user) + end + end + describe '#access_level=' do let(:user) { build(:user) } diff --git a/ee/spec/requests/smartcard_controller_spec.rb b/ee/spec/requests/smartcard_controller_spec.rb new file mode 100644 index 00000000000000..5d97839ef8f137 --- /dev/null +++ b/ee/spec/requests/smartcard_controller_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SmartcardController, type: :request do + let(:subject_dn) { '/O=Random Corp Ltd/CN=gitlab-user/emailAddress=gitlab-user@random-corp.org' } + let(:issuer_dn) { '/O=Random Corp Ltd/CN=Random Corp' } + let(:certificate_headers) { { 'X-SSL-CLIENT-CERTIFICATE': 'certificate' } } + let(:openssl_certificate_store) { instance_double(OpenSSL::X509::Store) } + let(:openssl_certificate) { instance_double(OpenSSL::X509::Certificate, subject: subject_dn, issuer: issuer_dn) } + let(:audit_event_service) { instance_double(AuditEventService) } + + subject { post '/-/smartcard/auth', {}, certificate_headers } + + describe '#auth' do + context 'with smartcard_auth enabled' do + before do + allow(Gitlab::Auth::Smartcard).to receive(:enabled?).and_return(true) + allow(Gitlab::Auth::Smartcard::Certificate).to receive(:store).and_return(openssl_certificate_store) + allow(OpenSSL::X509::Certificate).to receive(:new).and_return(openssl_certificate) + allow(openssl_certificate_store).to receive(:verify).and_return(true) + end + + it 'allows sign in' do + subject + + expect(request.env['warden']).to be_authenticated + end + + it 'redirects to root' do + subject + + expect(response).to redirect_to(root_url) + end + + it 'logs audit event' do + expect(AuditEventService).to( + receive(:new) + .with(instance_of(User), instance_of(User), with: 'smartcard') + .and_return(audit_event_service)) + expect(audit_event_service).to receive_message_chain(:for_authentication, :security_event) + + subject + end + + context 'user does not exist' do + context 'signup allowed' do + it 'creates user' do + expect { subject }.to change { User.count }.from(0).to(1) + end + end + + context 'signup disabled' do + it 'renders 401' do + allow(Gitlab::CurrentSettings.current_application_settings).to( + receive(:allow_signup?).and_return(false)) + + subject + + expect(flash[:alert]).to eql('Failed to signing using smartcard authentication') + expect(response).to redirect_to(new_user_session_path) + expect(request.env['warden']).not_to be_authenticated + end + end + end + + context 'user already exists' do + before do + user = create(:user) + create(:smartcard_identity, subject: subject_dn, issuer: issuer_dn, user: user) + end + + it 'finds existing user' do + expect { subject }.not_to change { User.count } + expect(request.env['warden']).to be_authenticated + end + end + + context 'missing certificate headers' do + let(:certificate_headers) { nil } + + it 'renders 401' do + subject + + expect(response).to have_gitlab_http_status(401) + expect(request.env['warden']).not_to be_authenticated + end + end + end + + context 'with smartcard_auth disabled' do + before do + allow(Gitlab::Auth::Smartcard).to receive(:enabled?).and_return(false) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8720f3aa93d5df..720df82c691c33 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3340,6 +3340,9 @@ msgstr "" msgid "Failed to remove the pipeline schedule" msgstr "" +msgid "Failed to signing using smartcard authentication" +msgstr "" + msgid "Failed to update issues, please try again." msgstr "" @@ -4887,6 +4890,9 @@ msgstr "" msgid "Locks give the ability to lock specific file or folder." msgstr "" +msgid "Login with smartcard" +msgstr "" + msgid "Logs" msgstr "" @@ -7462,6 +7468,12 @@ msgstr "" msgid "Slower but makes sure the project workspace is pristine as it clones the repository from scratch for every job" msgstr "" +msgid "Smartcard" +msgstr "" + +msgid "Smartcard authentication failed: client certificate header is missing." +msgstr "" + msgid "Snippets" msgstr "" diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 4e91068ab88300..efc3ce74627b90 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -650,7 +650,7 @@ def append_info_to_payload(payload) describe '#access_denied' do controller(described_class) do def index - access_denied!(params[:message]) + access_denied!(params[:message], params[:status]) end end @@ -669,6 +669,12 @@ def index expect(response).to have_gitlab_http_status(403) end + + it 'renders a status passed to access denied' do + get :index, status: 401 + + expect(response).to have_gitlab_http_status(401) + end end context 'when invalid UTF-8 parameters are received' do diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 44758f862a8188..ad856bd062e4e7 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -2,6 +2,7 @@ describe 'Login' do include TermsHelper + include UserLoginHelper before do stub_authentication_activity_metrics(debug: true) @@ -546,29 +547,6 @@ def sign_in_using_saml! ensure_tab_pane_correctness(false) end end - - def ensure_tab_pane_correctness(visit_path = true) - if visit_path - visit new_user_session_path - end - - ensure_tab_pane_counts - ensure_one_active_tab - ensure_one_active_pane - end - - def ensure_tab_pane_counts - tabs_count = page.all('[role="tab"]').size - expect(page).to have_selector('[role="tabpanel"]', count: tabs_count) - end - - def ensure_one_active_tab - expect(page).to have_selector('ul.new-session-tabs > li > a.active', count: 1) - end - - def ensure_one_active_pane - expect(page).to have_selector('.tab-pane.active', count: 1) - end end context 'when terms are enforced' do diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index 1d8952a58c5431..9e1b3332cb60aa 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -57,6 +57,16 @@ end end + describe 'form_based_auth_provider_has_active_class?' do + it 'selects main LDAP server' do + allow(helper).to receive(:auth_providers) { [:twitter, :ldapprimary, :ldapsecondary, :kerberos] } + expect(helper.form_based_auth_provider_has_active_class?(:twitter)).to be(false) + expect(helper.form_based_auth_provider_has_active_class?(:ldapprimary)).to be(true) + expect(helper.form_based_auth_provider_has_active_class?(:ldapsecondary)).to be(false) + expect(helper.form_based_auth_provider_has_active_class?(:kerberos)).to be(false) + end + end + describe 'enabled_button_based_providers' do before do allow(helper).to receive(:auth_providers) { [:twitter, :github] } diff --git a/spec/support/helpers/user_login_helper.rb b/spec/support/helpers/user_login_helper.rb new file mode 100644 index 00000000000000..36c002f53af65f --- /dev/null +++ b/spec/support/helpers/user_login_helper.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module UserLoginHelper + def ensure_tab_pane_correctness(visit_path = true) + if visit_path + visit new_user_session_path + end + + ensure_tab_pane_counts + ensure_one_active_tab + ensure_one_active_pane + end + + def ensure_tab_pane_counts + tabs_count = page.all('[role="tab"]').size + expect(page).to have_selector('[role="tabpanel"]', count: tabs_count) + end + + def ensure_one_active_tab + expect(page).to have_selector('ul.new-session-tabs > li > a.active', count: 1) + end + + def ensure_one_active_pane + expect(page).to have_selector('.tab-pane.active', count: 1) + end +end -- GitLab