diff --git a/app/controllers/admin/certificates_controller.rb b/app/controllers/admin/certificates_controller.rb index a6dd1c8bf..c2b6b5cc9 100644 --- a/app/controllers/admin/certificates_controller.rb +++ b/app/controllers/admin/certificates_controller.rb @@ -1,19 +1,21 @@ class Admin::CertificatesController < AdminController load_and_authorize_resource - before_action :set_api_user, only: [:new, :show, :destroy, :edit, :update] + before_action :set_certificate, :set_api_user, only: [:sign, :show, :download_csr, :download_crt, :revoke, :destroy] def show; end - def edit; end - def new + @api_user = ApiUser.find(params[:api_user_id]) @certificate = Certificate.new(api_user: @api_user) end def create @api_user = ApiUser.find(params[:api_user_id]) - @certificate = @api_user.certificates.build(certificate_params) + crt = certificate_params[:crt].open.read if certificate_params[:crt] + csr = certificate_params[:csr].open.read if certificate_params[:csr] + + @certificate = @api_user.certificates.build(csr: csr, crt: crt) if @api_user.save flash[:notice] = I18n.t('record_created') redirect_to [:admin, @api_user, @certificate] @@ -23,16 +25,6 @@ class Admin::CertificatesController < AdminController end end - def update - if @certificate.update(certificate_params) - flash[:notice] = I18n.t('record_updated') - redirect_to [:admin, @api_user, @certificate] - else - flash.now[:alert] = I18n.t('failed_to_update_record') - render 'edit' - end - end - def destroy if @certificate.destroy flash[:notice] = I18n.t('record_deleted') @@ -43,48 +35,50 @@ class Admin::CertificatesController < AdminController end end - # DEPRECATED FOR NOW - # def sign - # if @certificate.sign! - # flash[:notice] = I18n.t('record_updated') - # redirect_to [:admin, @api_user, @certificate] - # else - # flash.now[:alert] = I18n.t('failed_to_update_record') - # render 'show' - # end - # end + def sign + if @certificate.sign! + flash[:notice] = I18n.t('record_updated') + redirect_to [:admin, @api_user, @certificate] + else + flash.now[:alert] = I18n.t('failed_to_update_record') + render 'show' + end + end - # def revoke - # if @certificate.revoke! - # flash[:notice] = I18n.t('record_updated') - # else - # flash[:alert] = I18n.t('failed_to_update_record') - # end - # redirect_to [:admin, @api_user, @certificate] - # end + def revoke + if @certificate.revoke! + flash[:notice] = I18n.t('record_updated') + else + flash[:alert] = I18n.t('failed_to_update_record') + end + redirect_to [:admin, @api_user, @certificate] + end - # def download_csr - # send_data @certificate.csr, filename: "#{@api_user.username}.csr.pem" - # end + def download_csr + send_data @certificate.csr, filename: "#{@api_user.username}.csr.pem" + end - # def download_crt - # send_data @certificate.crt, filename: "#{@api_user.username}.crt.pem" - # end + def download_crt + send_data @certificate.crt, filename: "#{@api_user.username}.crt.pem" + end private - # DEPRECATED FOR NOW - # def set_certificate - # @certificate = Certificate.find(params[:id]) - # @csr = OpenSSL::X509::Request.new(@certificate.csr) if @certificate.csr - # @crt = OpenSSL::X509::Certificate.new(@certificate.crt) if @certificate.crt - # end + def set_certificate + @certificate = Certificate.find(params[:id]) + @csr = OpenSSL::X509::Request.new(@certificate.csr) if @certificate.csr + @crt = OpenSSL::X509::Certificate.new(@certificate.crt) if @certificate.crt + end def set_api_user @api_user = ApiUser.find(params[:api_user_id]) end def certificate_params - params.require(:certificate).permit(:common_name, :md5, :interface) + if params[:certificate] + params.require(:certificate).permit(:crt, :csr) + else + {} + end end end diff --git a/app/controllers/registrar/sessions_controller.rb b/app/controllers/registrar/sessions_controller.rb index f0d6fe549..9cda1e2ab 100644 --- a/app/controllers/registrar/sessions_controller.rb +++ b/app/controllers/registrar/sessions_controller.rb @@ -13,6 +13,7 @@ class Registrar::SessionsController < Devise::SessionsController # rubocop:disable Metrics/PerceivedComplexity # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/MethodLength def create @depp_user = Depp::User.new(params[:depp_user].merge( pki: !(Rails.env.development? || Rails.env.test?) @@ -23,17 +24,32 @@ class Registrar::SessionsController < Devise::SessionsController @depp_user.errors.add(:base, :webserver_missing_user_name_directive) end + if @depp_user.pki && request.env['HTTP_SSL_CLIENT_CERT'].blank? + @depp_user.errors.add(:base, :webserver_missing_client_cert_directive) + end + if @depp_user.pki && request.env['HTTP_SSL_CLIENT_S_DN_CN'] == '(null)' @depp_user.errors.add(:base, :webserver_user_name_directive_should_be_required) end - logger.error request.env - if @depp_user.pki && request.env['HTTP_SSL_CLIENT_S_DN_CN'] != params[:depp_user][:tag] - @depp_user.errors.add(:base, :invalid_cert) + if @depp_user.pki && request.env['HTTP_SSL_CLIENT_CERT'] == '(null)' + @depp_user.errors.add(:base, :webserver_client_cert_directive_should_be_required) + end + + @api_user = ApiUser.find_by(username: params[:depp_user][:tag], password: params[:depp_user][:password]) + + unless @api_user + @depp_user.errors.add(:base, t(:no_such_user)) + render 'login' and return + end + + if @depp_user.pki + unless @api_user.registrar_pki_ok?(request.env['HTTP_SSL_CLIENT_CERT']) + @depp_user.errors.add(:base, :invalid_cert) + end end if @depp_user.errors.none? && @depp_user.valid? - @api_user = ApiUser.find_by(username: params[:depp_user][:tag]) if @api_user.active? sign_in @api_user redirect_to role_base_root_url(@api_user) @@ -47,6 +63,7 @@ class Registrar::SessionsController < Devise::SessionsController end # rubocop:enable Metrics/CyclomaticComplexity # rubocop:enable Metrics/PerceivedComplexity + # rubocop:enable Metrics/MethodLength def id @user = ApiUser.find_by_idc_data(request.env['SSL_CLIENT_S_DN']) diff --git a/app/models/api_user.rb b/app/models/api_user.rb index 79810eeab..7b416ba8d 100644 --- a/app/models/api_user.rb +++ b/app/models/api_user.rb @@ -45,6 +45,14 @@ class ApiUser < User registrar.messages.queued end + def registrar_pki_ok?(crt) + certificates.registrar.exists?(crt: crt) + end + + def api_pki_ok?(crt) + certificates.api.exists?(crt: crt) + end + class << self def find_by_idc_data(idc_data) return false if idc_data.blank? diff --git a/app/models/certificate.rb b/app/models/certificate.rb index 1c3720958..8f0255210 100644 --- a/app/models/certificate.rb +++ b/app/models/certificate.rb @@ -11,9 +11,43 @@ class Certificate < ActiveRecord::Base REVOKED = 'revoked' VALID = 'valid' - INTERFACES = ['api', 'registrar'] + API = 'api' + REGISTRAR = 'registrar' + INTERFACES = [API, REGISTRAR] - validates :common_name, :md5, :interface, presence: true + scope 'api', -> { where(interface: API) } + scope 'registrar', -> { where(interface: REGISTRAR) } + + validate :validate_csr_and_crt_presence + def validate_csr_and_crt_presence + return if csr.try(:scrub).present? || crt.try(:scrub).present? + errors.add(:base, I18n.t(:crt_or_csr_must_be_present)) + end + + validate :validate_csr_and_crt + def validate_csr_and_crt + parsed_crt + parsed_csr + rescue OpenSSL::X509::RequestError, OpenSSL::X509::CertificateError + errors.add(:base, I18n.t(:invalid_csr_or_crt)) + end + + before_create :parse_metadata + def parse_metadata + if crt + pc = parsed_crt.try(:subject).try(:to_s) || '' + cn = pc.scan(/\/CN=(.+)/).flatten.first + self.common_name = cn.split('/').first + self.md5 = Digest::MD5.hexdigest(crt) + self.interface = API + elsif csr + pc = parsed_csr.try(:subject).try(:to_s) || '' + cn = pc.scan(/\/CN=(.+)/).flatten.first + self.common_name = cn.split('/').first + self.md5 = Digest::MD5.hexdigest(csr) + self.interface = REGISTRAR + end + end def parsed_crt @p_crt ||= OpenSSL::X509::Certificate.new(crt) if crt diff --git a/app/views/admin/api_users/show.haml b/app/views/admin/api_users/show.haml index 9e956690c..866084736 100644 --- a/app/views/admin/api_users/show.haml +++ b/app/views/admin/api_users/show.haml @@ -38,19 +38,24 @@ .pull-left = t(:certificates) .pull-right - = link_to(t(:add), - new_admin_api_user_certificate_path(@api_user), class: 'btn btn-default btn-xs') + = link_to(t(:upload_crt), + new_admin_api_user_certificate_path(@api_user, crt: true), class: 'btn btn-primary btn-xs') + = link_to(t(:upload_csr), + new_admin_api_user_certificate_path(@api_user), class: 'btn btn-primary btn-xs') .table-responsive %table.table.table-hover.table-bordered.table-condensed %thead %tr - %th{class: 'col-xs-4'}= t(:common_name) - %th{class: 'col-xs-4'}= t(:md5) - %th{class: 'col-xs-4'}= t(:interface) + %th{class: 'col-xs-10'}= t(:subject) + %th{class: 'col-xs-2'}= t(:status) %tbody - @api_user.certificates.each do |x| - %tr - %td= link_to(x.common_name, admin_api_user_certificate_path(@api_user, x)) - %td= x.md5 - %td= x.interface + - if x.csr + %tr + %td= link_to(x.parsed_csr.try(:subject), admin_api_user_certificate_path(@api_user, x)) + %td= x.status + - elsif x.crt + %tr + %td= link_to(x.parsed_crt.try(:subject), admin_api_user_certificate_path(@api_user, x)) + %td= x.status diff --git a/app/views/admin/certificates/_form.haml b/app/views/admin/certificates/_form.haml deleted file mode 100644 index 4f468dcaf..000000000 --- a/app/views/admin/certificates/_form.haml +++ /dev/null @@ -1,29 +0,0 @@ -= form_for([:admin, @api_user, @certificate], html: {class: 'form-horizontal'}) do |f| - = render 'shared/full_errors', object: f.object - - .row - .col-md-8 - .form-group - .col-md-4.control-label - = f.label :api_user - .col-md-7 - = f.text_field(:api_user, class: 'form-control', disabled: :disabled) - .form-group - .col-md-4.control-label - = f.label :common_name - .col-md-7 - = f.text_field(:common_name, class: 'form-control', autocomplete: 'off') - .form-group - .col-md-4.control-label - = f.label :md5 - .col-md-7 - = f.text_field(:md5, class: 'form-control', autocomplete: 'off') - .form-group - .col-md-4.control-label - = f.label :interface - .col-md-7 - = f.select :interface, Certificate::INTERFACES.map {|x| [x.upcase, x]}, {}, class: 'form-control selectize' - %hr - .row - .col-md-8.text-right - = button_tag(t(:save), class: 'btn btn-primary') diff --git a/app/views/admin/certificates/edit.haml b/app/views/admin/certificates/edit.haml deleted file mode 100644 index 17be6172f..000000000 --- a/app/views/admin/certificates/edit.haml +++ /dev/null @@ -1,5 +0,0 @@ -- content_for :actions do - = link_to(t(:back_to_api_user), admin_api_user_path(@api_user), class: 'btn btn-default') - -= render 'shared/title', name: t(:edit_certificate) -= render 'form' diff --git a/app/views/admin/certificates/new.haml b/app/views/admin/certificates/new.haml index 02a2e6052..40947ed19 100644 --- a/app/views/admin/certificates/new.haml +++ b/app/views/admin/certificates/new.haml @@ -1,5 +1,22 @@ -- content_for :actions do - = link_to(t(:back_to_api_user), admin_api_user_path(@api_user), class: 'btn btn-default') += render 'shared/title', name: params[:crt] ? t(:upload_crt) : t(:upload_csr) -= render 'shared/title', name: t(:add_certificate) -= render 'form' += form_for([:admin, @api_user, @certificate], multipart: true) do |f| + = render 'shared/full_errors', object: f.object + + .row + .col-md-8 + .form-group + - if params[:crt] + .col-md-4.control-label + = f.label :crt, t(:certificate) + .col-md-8 + = f.file_field :crt + - else + .col-md-4.control-label + = f.label :csr, t(:certificate_signing_req) + .col-md-8 + = f.file_field :csr + %hr + .row + .col-md-8.text-right + = button_tag(t(:save), class: 'btn btn-primary') diff --git a/app/views/admin/certificates/show.haml b/app/views/admin/certificates/show.haml index 2285c1a53..5adbdbdc4 100644 --- a/app/views/admin/certificates/show.haml +++ b/app/views/admin/certificates/show.haml @@ -1,5 +1,4 @@ - content_for :actions do - = link_to(t(:edit), edit_admin_api_user_certificate_path(@api_user, @certificate), class: 'btn btn-default') = link_to(t(:delete), admin_api_user_certificate_path(@api_user, @certificate), method: :delete, data: { confirm: t(:are_you_sure) }, class: 'btn btn-danger') = render 'shared/title', name: t(:certificates) @@ -30,10 +29,72 @@ %dd= @certificate.md5 %dt= t(:interface) - %dd= @certificate.interface + %dd= @certificate.interface.try(:upcase) %dt= t(:updated_at) %dd= l(@certificate.updated_at) %dt= t(:created_at) %dd= l(@certificate.created_at) + +- if @csr + .row + .col-md-12 + .panel.panel-default + .panel-heading.clearfix + .pull-left + = t(:csr) + .pull-right + = link_to(t(:download), download_csr_admin_api_user_certificate_path(@api_user, @certificate), class: 'btn btn-default btn-xs') + - unless @crt + = link_to(t(:sign_this_request), sign_admin_api_user_certificate_path(@api_user, @certificate), method: :post, class: 'btn btn-primary btn-xs') + + .panel-body + %dl.dl-horizontal + %dt= t(:version) + %dd= @csr.version + + %dt= t(:subject) + %dd= @csr.subject + + %dt= t(:signature_algorithm) + %dd= @csr.signature_algorithm + +- if @crt + .row + .col-md-12 + .panel.panel-default + .panel-heading.clearfix + .pull-left + = t('crt') unless @certificate.revoked? + = t('crt_revoked') if @certificate.revoked? + .pull-right + = link_to(t(:download), download_crt_admin_api_user_certificate_path(@api_user, @certificate), class: 'btn btn-default btn-xs') + - if !@certificate.revoked? && @certificate.csr + = link_to(t(:revoke_this_certificate), revoke_admin_api_user_certificate_path(@api_user, @certificate), method: :post, class: 'btn btn-primary btn-xs') + - if @crt + .panel-body + %dl.dl-horizontal + %dt= t(:version) + %dd= @crt.version + + %dt= t(:serial_number) + %dd= @crt.serial + + %dt= t(:signature_algorithm) + %dd= @crt.signature_algorithm + + %dt= t(:issuer) + %dd= @crt.issuer + + %dt= t(:valid_from) + %dd= @crt.not_before + + %dt= t(:valid_to) + %dd= @crt.not_after + + %dt= t(:subject) + %dd= @crt.subject + + %dt= t(:extensions) + %dd= @crt.extensions.map(&:to_s).join('
').html_safe diff --git a/config/locales/en.yml b/config/locales/en.yml index d6d400c33..4a4707f59 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -804,3 +804,6 @@ en: interface: 'Interface' add_certificate: 'Add certificate' edit_certificate: 'Edit certificate' + invalid_csr_or_crt: 'Invalid CSR or CRT' + webserver_missing_client_cert_directive: 'Webserver missing client cert directive' + webserver_client_cert_directive_should_be_required: 'Webserver client cert directive should be required' diff --git a/spec/features/registrar/sessions_spec.rb b/spec/features/registrar/sessions_spec.rb index 4a5c19a84..5d5f3095a 100644 --- a/spec/features/registrar/sessions_spec.rb +++ b/spec/features/registrar/sessions_spec.rb @@ -25,6 +25,14 @@ feature 'Sessions', type: :feature do click_button 'Log in' page.should have_text('Access denied') end + + it 'should not get in with invalid user' do + visit registrar_login_path + fill_in 'depp_user_tag', with: 'bla' + fill_in 'depp_user_password', with: 'bla' + click_button 'Log in' + page.should have_text('No such user') + end end context 'as unknown user' do diff --git a/spec/models/certificate_spec.rb b/spec/models/certificate_spec.rb index 53e080e04..232492269 100644 --- a/spec/models/certificate_spec.rb +++ b/spec/models/certificate_spec.rb @@ -11,9 +11,7 @@ describe Certificate do it 'should not be valid' do @certificate.valid? @certificate.errors.full_messages.should match_array([ - "Common name is missing", - "Interface is missing", - "Md5 is missing" + "CRT or CSR must be present" ]) end