From 931d4a7db02deebf5a630fb84faef096709cbfd8 Mon Sep 17 00:00:00 2001 From: "konstantin.grabar" Date: Tue, 31 Jul 2012 16:50:53 +0400 Subject: [PATCH] [refs #441] Key Pairs fixes and refactoring --- .../platforms/key_pairs_controller.rb | 11 ++++------ app/models/key_pair.rb | 4 ++-- app/models/repository.rb | 2 +- config/locales/models/key_pair.en.yml | 7 ++++--- config/locales/models/key_pair.ru.yml | 9 +++++---- .../platforms/key_pairs_controller_spec.rb | 20 +++++++++++++++---- spec/spec_helper.rb | 6 ++++++ 7 files changed, 38 insertions(+), 21 deletions(-) diff --git a/app/controllers/platforms/key_pairs_controller.rb b/app/controllers/platforms/key_pairs_controller.rb index c45f53dcb..4d9fb0ab0 100644 --- a/app/controllers/platforms/key_pairs_controller.rb +++ b/app/controllers/platforms/key_pairs_controller.rb @@ -1,11 +1,8 @@ class Platforms::KeyPairsController < ApplicationController before_filter :authenticate_user! - load_and_authorize_resource :platform - load_and_authorize_resource - - skip_load_and_authorize_resource :only => [:index] - skip_authorize_resource :platform, :only => [:create, :destroy] + load_and_authorize_resource :platform, :only => [:index] + load_and_authorize_resource :only => [:create, :destroy] def create @key_pair.user_id = current_user.id @@ -17,7 +14,7 @@ class Platforms::KeyPairsController < ApplicationController flash[:warning] = @key_pair.errors.full_messages.join('. ') unless @key_pair.errors.blank? end - redirect_to platform_key_pairs_path(@platform) + redirect_to platform_key_pairs_path(@key_pair.repository.platform) end def destroy @@ -27,6 +24,6 @@ class Platforms::KeyPairsController < ApplicationController flash[:error] = t('flash.key_pairs.destroy_error') end - redirect_to platform_key_pairs_path(@platform) + redirect_to platform_key_pairs_path(@key_pair.repository.platform) end end diff --git a/app/models/key_pair.rb b/app/models/key_pair.rb index 35fe32203..ce6c77365 100644 --- a/app/models/key_pair.rb +++ b/app/models/key_pair.rb @@ -18,8 +18,8 @@ class KeyPair < ActiveRecord::Base def key_create_call result, self.key_id = BuildServer.import_gpg_key_pair(public, secret) raise "Failed to create key_pairs for repository #{repository_id} with code #{result}." if result == 4 - if result != 0 || key_id.nil? - errors.add(:public, I18n.t("activerecord.errors.key_pair.#{result}")) + if result != 0 || self.key_id.nil? + errors.add(:public, I18n.t("activerecord.errors.key_pair.rpc_error_#{result}")) return false end result = BuildServer.set_repository_key(repository.platform.name, repository.name, key_id) diff --git a/app/models/repository.rb b/app/models/repository.rb index 19f4c8888..491d219e8 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -4,7 +4,7 @@ class Repository < ActiveRecord::Base has_many :project_to_repositories, :dependent => :destroy, :validate => true has_many :projects, :through => :project_to_repositories - has_one :key_pair + has_one :key_pair, :dependent => :destroy validates :description, :presence => true validates :name, :uniqueness => {:scope => :platform_id, :case_sensitive => false}, :presence => true, :format => {:with => /^[a-z0-9_\-]+$/} diff --git a/config/locales/models/key_pair.en.yml b/config/locales/models/key_pair.en.yml index 139c9da5a..9b685612a 100644 --- a/config/locales/models/key_pair.en.yml +++ b/config/locales/models/key_pair.en.yml @@ -17,9 +17,10 @@ en: errors: key_pair: repo_key_exists: Repository has been signed already! Please remove old signature and try again - 1: could not import public key - 2: could not import secret key - 3: keys are imported, but it is not a key pair (ids differ) + rpc_error_0: an unexpected error + rpc_error_1: could not import public key + rpc_error_2: could not import secret key + rpc_error_3: keys are imported, but it is not a key pair (ids differ) models: key_pair: Key Pair attributes: diff --git a/config/locales/models/key_pair.ru.yml b/config/locales/models/key_pair.ru.yml index 0b2fb496f..465ab0052 100644 --- a/config/locales/models/key_pair.ru.yml +++ b/config/locales/models/key_pair.ru.yml @@ -17,9 +17,10 @@ ru: errors: key_pair: repo_key_exists: Репозиторий уже подписан! Пожалуйста, удалите старую подпись и попробуйте снова - 1: Проблемы с импортром публичного ключа - 2: Проблемы с импортром секретного ключа - 3: Ключи импортированы, но не являются парой (идентификаторы не совпадают) + rpc_error_0: Неизвестная ошибка + rpc_error_1: Проблемы с импортром публичного ключа + rpc_error_2: Проблемы с импортром секретного ключа + rpc_error_3: Ключи импортированы, но не являются парой (идентификаторы не совпадают) models: key_pair: Подпись attributes: @@ -31,4 +32,4 @@ ru: repository_id: Репозиторий public: Публичный ключ secret: Секретный ключ - key_id: Подпись \ No newline at end of file + key_id: Подпись diff --git a/spec/controllers/platforms/key_pairs_controller_spec.rb b/spec/controllers/platforms/key_pairs_controller_spec.rb index 7beeec068..5b27e41b0 100644 --- a/spec/controllers/platforms/key_pairs_controller_spec.rb +++ b/spec/controllers/platforms/key_pairs_controller_spec.rb @@ -69,6 +69,7 @@ end describe Platforms::KeyPairsController do before(:each) do stub_symlink_methods + stub_key_pairs_calls @platform = FactoryGirl.create(:platform) @repository = FactoryGirl.create(:repository, :platform => @platform) @@ -84,9 +85,9 @@ describe Platforms::KeyPairsController do end context 'for guest' do - [:index, :create, :destroy].each do |action| + [:index, :create].each do |action| it "should not be able to perform #{ action } action" do - get action, :platform_id => @platform, :id => @key_pair + get action, :platform_id => @platform response.should redirect_to(new_user_session_path) end end @@ -95,8 +96,19 @@ describe Platforms::KeyPairsController do lambda { post :create, @create_params }.should change{ KeyPair.count }.by(0) end - it 'should not change objects count on destroy success' do - lambda { delete :destroy, :platform_id => @platform, :id => @key_pair }.should change{KeyPair.count}.by(0) + context 'on destroy' do + before(:each) do + create_key_pair @repository, @user + end + + it 'should not change objects count on destroy success' do + lambda { delete :destroy, :platform_id => @platform, :id => @key_pair }.should change{KeyPair.count}.by(0) + end + + it "should not be able to perform destroy action" do + delete :destroy, :platform_id => @platform, :id => @key_pair + response.should redirect_to(new_user_session_path) + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 72f943b66..241d36fc9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -38,6 +38,12 @@ def stub_symlink_methods any_instance_of(Platform, :remove_symlink_directory => true) end +def stub_key_pairs_calls + stub(BuildServer).import_gpg_key_pair { [0,0] } + stub(BuildServer).set_repository_key { 0 } + stub(BuildServer).rm_repository_key { 0 } +end + def test_git_commit(project) project.repo.index.add('test', 'TEST') project.repo.index.commit('Test commit')