From 9ad66c0999970440d4b3094632d5122a37775951 Mon Sep 17 00:00:00 2001 From: Martin Lensment Date: Thu, 21 May 2015 17:06:21 +0300 Subject: [PATCH] Add CN support to certificates --- .../admin/certificates_controller.rb | 82 +++++++++++-------- app/models/certificate.rb | 7 +- app/views/admin/api_users/show.haml | 25 +++--- app/views/admin/certificates/_form.haml | 29 +++++++ app/views/admin/certificates/edit.haml | 5 ++ app/views/admin/certificates/new.haml | 26 +----- app/views/admin/certificates/show.haml | 79 ++++++------------ config/locales/en.yml | 5 ++ ...150521120145_add_fields_for_certificate.rb | 7 ++ db/schema.rb | 5 +- spec/fabricators/certificate_fabricator.rb | 3 + spec/models/certificate_spec.rb | 4 +- 12 files changed, 144 insertions(+), 133 deletions(-) create mode 100644 app/views/admin/certificates/_form.haml create mode 100644 app/views/admin/certificates/edit.haml create mode 100644 db/migrate/20150521120145_add_fields_for_certificate.rb diff --git a/app/controllers/admin/certificates_controller.rb b/app/controllers/admin/certificates_controller.rb index df878eec8..a6dd1c8bf 100644 --- a/app/controllers/admin/certificates_controller.rb +++ b/app/controllers/admin/certificates_controller.rb @@ -1,21 +1,19 @@ class Admin::CertificatesController < AdminController load_and_authorize_resource - before_action :set_certificate, :set_api_user, only: [:sign, :show, :download_csr, :download_crt, :revoke] + before_action :set_api_user, only: [:new, :show, :destroy, :edit, :update] def show; end + def edit; end + def new - set_api_user - @certificate = Certificate.new + @certificate = Certificate.new(api_user: @api_user) end def create @api_user = ApiUser.find(params[:api_user_id]) - 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) + @certificate = @api_user.certificates.build(certificate_params) if @api_user.save flash[:notice] = I18n.t('record_created') redirect_to [:admin, @api_user, @certificate] @@ -25,50 +23,68 @@ class Admin::CertificatesController < AdminController end end - def sign - if @certificate.sign! + 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') + redirect_to admin_api_user_path(@api_user) + else + flash.now[:alert] = I18n.t('failed_to_delete_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 + # 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 download_csr - send_data @certificate.csr, filename: "#{@api_user.username}.csr.pem" - 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_crt - send_data @certificate.crt, filename: "#{@api_user.username}.crt.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 private - 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 + # 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_api_user @api_user = ApiUser.find(params[:api_user_id]) end def certificate_params - if params[:certificate] - params.require(:certificate).permit(:csr, :crt) - else - {} - end + params.require(:certificate).permit(:common_name, :md5, :interface) end end diff --git a/app/models/certificate.rb b/app/models/certificate.rb index a6fdd9b6b..1c3720958 100644 --- a/app/models/certificate.rb +++ b/app/models/certificate.rb @@ -11,12 +11,9 @@ class Certificate < ActiveRecord::Base REVOKED = 'revoked' VALID = 'valid' - validate :validate_csr_and_crt + INTERFACES = ['api', 'registrar'] - def validate_csr_and_crt - return if csr.present? || crt.present? - errors.add(:base, I18n.t(:crt_or_csr_must_be_present)) - end + validates :common_name, :md5, :interface, presence: true 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 87e6bce5c..9e956690c 100644 --- a/app/views/admin/api_users/show.haml +++ b/app/views/admin/api_users/show.haml @@ -1,5 +1,5 @@ - content_for :actions do - = link_to(t(:edit), edit_admin_api_user_path(@api_user), class: 'btn btn-primary') + = link_to(t(:edit), edit_admin_api_user_path(@api_user), class: 'btn btn-default') = link_to(t(:delete), admin_api_user_path(@api_user), method: :delete, data: { confirm: t(:are_you_sure) }, class: 'btn btn-danger') = render 'shared/title', name: @api_user.username @@ -38,24 +38,19 @@ .pull-left = t(:certificates) .pull-right - = 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') + = link_to(t(:add), + new_admin_api_user_certificate_path(@api_user), class: 'btn btn-default btn-xs') .table-responsive %table.table.table-hover.table-bordered.table-condensed %thead %tr - %th{class: 'col-xs-10'}= t(:subject) - %th{class: 'col-xs-2'}= t(:status) + %th{class: 'col-xs-4'}= t(:common_name) + %th{class: 'col-xs-4'}= t(:md5) + %th{class: 'col-xs-4'}= t(:interface) %tbody - @api_user.certificates.each do |x| - - 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 + %tr + %td= link_to(x.common_name, admin_api_user_certificate_path(@api_user, x)) + %td= x.md5 + %td= x.interface diff --git a/app/views/admin/certificates/_form.haml b/app/views/admin/certificates/_form.haml new file mode 100644 index 000000000..4f468dcaf --- /dev/null +++ b/app/views/admin/certificates/_form.haml @@ -0,0 +1,29 @@ += 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 new file mode 100644 index 000000000..17be6172f --- /dev/null +++ b/app/views/admin/certificates/edit.haml @@ -0,0 +1,5 @@ +- 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 60ece69d4..02a2e6052 100644 --- a/app/views/admin/certificates/new.haml +++ b/app/views/admin/certificates/new.haml @@ -1,23 +1,5 @@ -= render 'shared/title', name: params[:crt] ? t(:upload_crt) : t(:upload_csr) - -= 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') +- 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(:add_certificate) += render 'form' diff --git a/app/views/admin/certificates/show.haml b/app/views/admin/certificates/show.haml index dc66b41c7..2285c1a53 100644 --- a/app/views/admin/certificates/show.haml +++ b/app/views/admin/certificates/show.haml @@ -1,5 +1,7 @@ - content_for :actions do - = link_to(t(:back_to_api_user), [:admin, @api_user], class: 'btn btn-default') + = 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) - if @certificate.errors.any? @@ -9,64 +11,29 @@ - if @certificate.errors.any? %hr -- 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') +.row + .col-md-12 + .panel.panel-default + .panel-heading.clearfix + .pull-left + = t(:general) - .panel-body - %dl.dl-horizontal - %dt= t(:version) - %dd= @csr.version + .panel-body + %dl.dl-horizontal + %dt= t(:api_user) + %dd= link_to(@certificate.api_user, [:admin, @api_user]) - %dt= t(:subject) - %dd= @csr.subject + %dt= t(:common_name) + %dd= @certificate.common_name - %dt= t(:signature_algorithm) - %dd= @csr.signature_algorithm + %dt= t(:md5) + %dd= @certificate.md5 -- 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(:interface) + %dd= @certificate.interface - %dt= t(:serial_number) - %dd= @crt.serial + %dt= t(:updated_at) + %dd= l(@certificate.updated_at) - %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 + %dt= t(:created_at) + %dd= l(@certificate.created_at) diff --git a/config/locales/en.yml b/config/locales/en.yml index 7997ae15e..d6d400c33 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -799,3 +799,8 @@ en: no_permission: 'No permission' access_denied: 'Access denied' connection_limit_reached: 'Connection limit reached' + common_name: 'Common name' + md5: 'Md5' + interface: 'Interface' + add_certificate: 'Add certificate' + edit_certificate: 'Edit certificate' diff --git a/db/migrate/20150521120145_add_fields_for_certificate.rb b/db/migrate/20150521120145_add_fields_for_certificate.rb new file mode 100644 index 000000000..dc52f5fa1 --- /dev/null +++ b/db/migrate/20150521120145_add_fields_for_certificate.rb @@ -0,0 +1,7 @@ +class AddFieldsForCertificate < ActiveRecord::Migration + def change + add_column :certificates, :common_name, :string + add_column :certificates, :md5, :string + add_column :certificates, :interface, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index d4a2806b3..5fb053cb5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150520164507) do +ActiveRecord::Schema.define(version: 20150521120145) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -147,6 +147,9 @@ ActiveRecord::Schema.define(version: 20150520164507) do t.string "updator_str" t.datetime "created_at" t.datetime "updated_at" + t.string "common_name" + t.string "md5" + t.string "interface" end add_index "certificates", ["api_user_id"], name: "index_certificates_on_api_user_id", using: :btree diff --git a/spec/fabricators/certificate_fabricator.rb b/spec/fabricators/certificate_fabricator.rb index 8e97b9ec5..804543c39 100644 --- a/spec/fabricators/certificate_fabricator.rb +++ b/spec/fabricators/certificate_fabricator.rb @@ -1,6 +1,9 @@ # default fabricator should be reusable Fabricator(:certificate) do api_user + common_name 'cn' + md5 'md5hash' + interface 'api' csr "-----BEGIN CERTIFICATE REQUEST-----\n" \ "MIIE+DCCAuACAQAwgZ0xCzAJBgNVBAYTAkVFMREwDwYDVQQIDAhIYXJqdW1hYTEQ\n" \ "MA4GA1UEBwwHVGFsbGlubjEbMBkGA1UECgwSRWVzdGkgSW50ZXJuZXRpIFNBMRIw\n" \ diff --git a/spec/models/certificate_spec.rb b/spec/models/certificate_spec.rb index 232492269..53e080e04 100644 --- a/spec/models/certificate_spec.rb +++ b/spec/models/certificate_spec.rb @@ -11,7 +11,9 @@ describe Certificate do it 'should not be valid' do @certificate.valid? @certificate.errors.full_messages.should match_array([ - "CRT or CSR must be present" + "Common name is missing", + "Interface is missing", + "Md5 is missing" ]) end