Merge branch 'master' of github.com:domify/registry

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
-  # def sign
+    if @certificate.sign!
-  #   if @certificate.sign!
+      flash[:notice] = I18n.t('record_updated')
-  #     flash[:notice] = I18n.t('record_updated')
+      redirect_to [:admin, @api_user, @certificate]
-  #     redirect_to [:admin, @api_user, @certificate]
+    else
-  #   else
+      flash.now[:alert] = I18n.t('failed_to_update_record')
-  #     flash.now[:alert] = I18n.t('failed_to_update_record')
+      render 'show'
-  #     render 'show'
+    end
-  #   end
+  end
-  # end
 
-  # def revoke
+  def revoke
-  #   if @certificate.revoke!
+    if @certificate.revoke!
-  #     flash[:notice] = I18n.t('record_updated')
+      flash[:notice] = I18n.t('record_updated')
-  #   else
+    else
-  #     flash[:alert] = I18n.t('failed_to_update_record')
+      flash[:alert] = I18n.t('failed_to_update_record')
-  #   end
+    end
-  #   redirect_to [:admin, @api_user, @certificate]
+    redirect_to [:admin, @api_user, @certificate]
-  # end
+  end
 
-  # def download_csr
+  def download_csr
-  #   send_data @certificate.csr, filename: "#{@api_user.username}.csr.pem"
+    send_data @certificate.csr, filename: "#{@api_user.username}.csr.pem"
-  # end
+  end
 
-  # def download_crt
+  def download_crt
-  #   send_data @certificate.crt, filename: "#{@api_user.username}.crt.pem"
+    send_data @certificate.crt, filename: "#{@api_user.username}.crt.pem"
-  # end
+  end
 
   private
 
-  # DEPRECATED FOR NOW
+  def set_certificate
-  # def set_certificate
+    @certificate = Certificate.find(params[:id])
-  #   @certificate = Certificate.find(params[:id])
+    @csr = OpenSSL::X509::Request.new(@certificate.csr) if @certificate.csr
-  #   @csr = OpenSSL::X509::Request.new(@certificate.csr) if @certificate.csr
+    @crt = OpenSSL::X509::Certificate.new(@certificate.crt) if @certificate.crt
-  #   @crt = OpenSSL::X509::Certificate.new(@certificate.crt) if @certificate.crt
+  end
-  # 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

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_CERT'] == '(null)'
-    if @depp_user.pki && request.env['HTTP_SSL_CLIENT_S_DN_CN'] != params[:depp_user][:tag]
+      @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'])

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?

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

app/views/admin/api_users/show.haml

@@ -38,19 +38,24 @@
         .pull-left
           = t(:certificates)
         .pull-right
-          = link_to(t(:add),
+          = link_to(t(:upload_crt),
-            new_admin_api_user_certificate_path(@api_user), class: 'btn btn-default btn-xs')
+            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-10'}= t(:subject)
-              %th{class: 'col-xs-4'}= t(:md5)
+              %th{class: 'col-xs-2'}= t(:status)
-              %th{class: 'col-xs-4'}= t(:interface)
           %tbody
             - @api_user.certificates.each do |x|
+              - if x.csr
                 %tr
-                %td= link_to(x.common_name, admin_api_user_certificate_path(@api_user, x))
+                  %td= link_to(x.parsed_csr.try(:subject), admin_api_user_certificate_path(@api_user, x))
-                %td= x.md5
+                  %td= x.status
-                %td= x.interface
+              - elsif x.crt
+                %tr
+                  %td= link_to(x.parsed_crt.try(:subject), admin_api_user_certificate_path(@api_user, x))
+                  %td= x.status

app/views/admin/certificates/_form.haml

@@ -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')

app/views/admin/certificates/edit.haml

@@ -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'

app/views/admin/certificates/new.haml

@@ -1,5 +1,22 @@
-- content_for :actions do
+= render 'shared/title', name: params[:crt] ? t(:upload_crt) : t(:upload_csr)
-  = link_to(t(:back_to_api_user), admin_api_user_path(@api_user), class: 'btn btn-default')
 
-= render 'shared/title', name: t(:add_certificate)
+= form_for([:admin, @api_user, @certificate], multipart: true) do |f|
-= render 'form'
+  = 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')

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('<br>').html_safe

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'

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

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",
+        "CRT or CSR must be present"
-        "Interface is missing",
-        "Md5 is missing"
       ])
     end