Skip to content

Commit

Permalink
Merge pull request #70 from centosadmin/develop
Browse files Browse the repository at this point in the history
Release 1.7.0
  • Loading branch information
vladislav-yashin authored Dec 26, 2018
2 parents 2746ff8 + a0b4484 commit 902c913
Show file tree
Hide file tree
Showing 26 changed files with 129 additions and 43 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
Gemfile.lock
/.idea

11 changes: 9 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
language: ruby
rvm:
- 2.3.0
- 2.3.8
- 2.6.0

addons:
postgresql: "9.4"

env:
- REDMINE_VER=3.3-stable
- REDMINE_VER=3.4-stable
- REDMINE_VER=4.0-stable

matrix:
exclude:
- rvm: 2.6.0
env: REDMINE_VER=3.4-stable

install: "echo skip bundle install"

Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 1.7.0

* Redmine 4 support

# 1.6.0

* Depend on redmine_bots instead of redmine_telegram_common
Expand Down
5 changes: 3 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
source 'https://rubygems.org'

gem 'active_model_otp'
gem 'rotp', '~> 3.3.0'
gem 'rqrcode'

group :test do
gem 'timecop'
gem 'vcr'
gem 'webmock'
gem 'shoulda', '<= 3.5.0'
gem 'shoulda', '~> 3.6'
gem 'rails-controller-testing'
end
4 changes: 2 additions & 2 deletions app/controllers/otp_codes_controller.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
class OtpCodesController < ApplicationController
unloadable

skip_before_filter :check_if_login_required
before_filter :set_user_from_session
skip_before_action :check_if_login_required
before_action :set_user_from_session

def create # resend
send_code(@user)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/user_mobile_phone_controller.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
class UserMobilePhoneController < ApplicationController
unloadable

skip_before_filter :check_if_login_required
skip_before_action :check_if_login_required

before_filter :set_user_from_session
before_action :set_user_from_session

def update
@user.mobile_phone = params[:user][:mobile_phone]
Expand Down
1 change: 0 additions & 1 deletion app/models/redmine_2fa/telegram_account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ class Redmine2FA::TelegramAccount < ActiveRecord::Base
unloadable

belongs_to :user
attr_accessible :user_id, :first_name, :last_name, :username, :active, :telegram_id

before_save :set_token

Expand Down
2 changes: 1 addition & 1 deletion db/migrate/001_create_telegram_accounts.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class CreateTelegramAccounts < ActiveRecord::Migration
class CreateTelegramAccounts < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def change
create_table :redmine_2fa_telegram_accounts do |t|
t.integer :telegram_id
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/002_change_auth_source_limit.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class ChangeAuthSourceLimit < ActiveRecord::Migration
class ChangeAuthSourceLimit < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def up
change_column :auth_sources, :type, :string, limit: 40
end
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/003_add_otp_secret_key_to_users.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class AddOtpSecretKeyToUsers < ActiveRecord::Migration
class AddOtpSecretKeyToUsers < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def up
add_column :users, :otp_secret_key, :string
User.find_each { |user| user.update_attribute(:otp_secret_key, ROTP::Base32.random_base32) }
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/004_add_token_to_telegram_account.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class AddTokenToTelegramAccount < ActiveRecord::Migration
class AddTokenToTelegramAccount < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def up
add_column :redmine_2fa_telegram_accounts, :token, :string
Redmine2FA::TelegramAccount.find_each { |telegram_account| telegram_account.update_attribute(:token, ROTP::Base32.random_base32) }
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/005_add_mobile_phone_to_users.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class AddMobilePhoneToUsers < ActiveRecord::Migration
class AddMobilePhoneToUsers < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def up
add_column :users, :mobile_phone, :string unless column_exists? :users, :mobile_phone
end
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/006_add_auth_sources.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class AddAuthSources < ActiveRecord::Migration
class AddAuthSources < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def up
Redmine2FA::AuthSource::Telegram.create name: 'Telegram', onthefly_register: false, tls: false
Redmine2FA::AuthSource::GoogleAuth.create name: 'Google Auth', onthefly_register: false, tls: false
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/007_add_mobile_phone_confirmed_to_users.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class AddMobilePhoneConfirmedToUsers < ActiveRecord::Migration
class AddMobilePhoneConfirmedToUsers < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def change
add_column :users, :mobile_phone_confirmed, :boolean, default: false, null: false
end
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/008_add_ignore_tfa_to_users.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class AddIgnoreTfaToUsers < ActiveRecord::Migration
class AddIgnoreTfaToUsers < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def change
add_column :users, :ignore_2fa, :boolean, default: false, null: false
end
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/009_add_2fa_to_users.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class Add2faToUsers < ActiveRecord::Migration
class Add2faToUsers < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def change
add_column :users, :two_fa_id, :integer, index: true
end
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/010_create_telegram_connections.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class CreateTelegramConnections < ActiveRecord::Migration
class CreateTelegramConnections < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def change
create_table :redmine_2fa_telegram_connections do |t|
t.belongs_to :user, index: true, foreign_key: true
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/011_transfer_telegram_connections.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class TransferTelegramConnections < ActiveRecord::Migration
class TransferTelegramConnections < Rails.version < '5.0' ? ActiveRecord::Migration : ActiveRecord::Migration[4.2]
def up
User.where(two_fa_id: Redmine2FA::AuthSource::Telegram.first.id).each do |user|
next unless user.telegram_account
Expand Down
4 changes: 2 additions & 2 deletions init.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@

Redmine::Plugin.register :redmine_2fa do
name 'Redmine 2FA'
version '1.6.0'
version '1.7.0'
url 'https://github.com/centosadmin/redmine_2fa'
description 'Two-factor authorization for Redmine'
author 'Southbridge'
author_url 'https://github.com/centosadmin/redmine_2fa'

requires_redmine version_or_higher: '3.0'

requires_redmine_plugin :redmine_bots, '0.1.0'
requires_redmine_plugin :redmine_bots, '0.2.0'

settings(default: { 'required' => false,
'active_protocols' => Redmine2FA::AVAILABLE_PROTOCOLS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.included(base) # :nodoc:
base.class_eval do
unloadable

before_filter :set_user_from_session, only: [:confirm_otp, :confirm_2fa]
before_action :set_user_from_session, only: [:confirm_otp, :confirm_2fa]
end
end

Expand Down
8 changes: 3 additions & 5 deletions lib/redmine_2fa/patches/user_patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module Redmine2FA
module Patches
module UserPatch
def self.included(base)
base.send(:include, InstanceMethods)
base.prepend InstanceMethods
base.safe_attributes 'mobile_phone', 'ignore_2fa', 'two_fa_id'
base.validates_format_of :mobile_phone, with: /\A\d*\z/, allow_blank: true

Expand All @@ -11,19 +11,17 @@ def self.included(base)

has_one_time_password length: 6

alias_method_chain :update_hashed_password, :otp_auth

belongs_to :two_fa, class_name: 'AuthSource'
has_one :telegram_connection, class_name: 'Redmine2FA::TelegramConnection'
end
end

module InstanceMethods
def update_hashed_password_with_otp_auth
def update_hashed_password
if two_factor_authenticable?
salt_password(password) if password
else
update_hashed_password_without_otp_auth
super
end
end

Expand Down
40 changes: 33 additions & 7 deletions test/functional/account_controller_patch_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@ class AccountControllerPatchTest < ActionController::TestCase

context 'user without 2fa' do
context 'with valid login data' do
setup { post :login, username: 'jsmith', password: 'jsmith', back_url: 'http://test.host/' }
setup do
if Rails.version < '5.0'
post :login, username: 'jsmith', password: 'jsmith', back_url: 'http://test.host/'
else
post :login, params: { username: 'jsmith', password: 'jsmith', back_url: 'http://test.host/' }
end
end

context 'prepare' do
should set_session[:otp_user_id].to(2)
Expand All @@ -33,7 +39,11 @@ class AccountControllerPatchTest < ActionController::TestCase
context 'with invalid password' do
setup do
AccountController.any_instance.expects(:invalid_credentials)
post :login, username: 'jsmith', password: 'wrong', back_url: 'http://test.host/'
if Rails.version < '5.0'
post :login, username: 'jsmith', password: 'wrong', back_url: 'http://test.host/'
else
post :login, params: { username: 'jsmith', password: 'wrong', back_url: 'http://test.host/' }
end
end

context 'prepare' do
Expand All @@ -45,7 +55,11 @@ class AccountControllerPatchTest < ActionController::TestCase
context 'with invalid login' do
setup do
AccountController.any_instance.expects(:invalid_credentials)
post :login, username: 'invalid', password: 'wrong', back_url: 'http://test.host/'
if Rails.version < '5.0'
post :login, username: 'invalid', password: 'wrong', back_url: 'http://test.host/'
else
post :login, params: { username: 'invalid', password: 'wrong', back_url: 'http://test.host/' }
end
end

context 'prepare' do
Expand All @@ -59,7 +73,11 @@ class AccountControllerPatchTest < ActionController::TestCase
context 'google auth' do
setup do
User.any_instance.stubs(:two_fa).returns(auth_sources(:google_auth))
post :login, username: 'jsmith', password: 'jsmith'
if Rails.version < '5.0'
post :login, username: 'jsmith', password: 'jsmith'
else
post :login, params: { username: 'jsmith', password: 'jsmith' }
end
end
should render_template('account/otp')
end
Expand Down Expand Up @@ -91,7 +109,11 @@ class AccountControllerPatchTest < ActionController::TestCase

Redmine2FA::CodeSender.any_instance.expects(:send_code)

post :confirm_2fa, protocol: @auth_source.protocol
if Rails.version < '5.0'
post :confirm_2fa, protocol: @auth_source.protocol
else
post :confirm_2fa, params: { protocol: @auth_source.protocol }
end

assert_template 'account/otp'

Expand All @@ -104,7 +126,11 @@ class AccountControllerPatchTest < ActionController::TestCase
should 'not update auth source' do
@request.session[:otp_user_id] = nil

post :confirm_2fa, protocol: @auth_source.protocol
if Rails.version < '5.0'
post :confirm_2fa, protocol: @auth_source.protocol
else
post :confirm_2fa, params: { protocol: @auth_source.protocol }
end

@user.reload

Expand All @@ -115,4 +141,4 @@ class AccountControllerPatchTest < ActionController::TestCase

context 'confirm one time password' do
end
end
end
20 changes: 17 additions & 3 deletions test/functional/second_authentications_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ class SecondAuthenticationsControllerTest < ActionController::TestCase

assert_equal @user_self.two_fa, @auth_source

delete :destroy, id: @user_self.id
if Rails.version < '5.0'
delete :destroy, id: @user_self.id
else
delete :destroy, params: { id: @user_self.id }
end

assert_response 302

@user_self.reload
Expand All @@ -38,7 +43,12 @@ class SecondAuthenticationsControllerTest < ActionController::TestCase

assert_equal @user_self.two_fa, @auth_source

delete :destroy, id: @user_self.id
if Rails.version < '5.0'
delete :destroy, id: @user_self.id
else
delete :destroy, params: { id: @user_self.id }
end

assert_response 302

@user_self.reload
Expand All @@ -49,7 +59,11 @@ class SecondAuthenticationsControllerTest < ActionController::TestCase
User.current = @user_other
@request.session[:user_id] = @user_other.id
assert_equal @user_self.two_fa, @auth_source
delete :destroy, id: @user_self.id
if Rails.version < '5.0'
delete :destroy, id: @user_self.id
else
delete :destroy, params: { id: @user_self.id }
end
assert_response 403

@user_self.reload
Expand Down
24 changes: 20 additions & 4 deletions test/functional/user_mobile_phone_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ def setup
def test_user_phone_update
@request.session[:otp_user_id] = @user.id
User.any_instance.expects(:otp_code).returns('123456')
post :update, user: { mobile_phone: '79241234567' }
if Rails.version < '5.0'
post :update, user: { mobile_phone: '79241234567' }, format: 'js'
else
post :update, params: { user: { mobile_phone: '79241234567' } }, format: 'js'
end

assert_equal @user, assigns(:user)

Expand All @@ -20,7 +24,11 @@ def test_user_phone_update
end

def test_user_phone_update_unauthorized
post :update, user: { mobile_phone: '5555555' }
if Rails.version < '5.0'
post :update, user: { mobile_phone: '5555555' }
else
post :update, params: { user: { mobile_phone: '5555555' } }
end

@user.reload

Expand All @@ -36,7 +44,11 @@ def test_user_phone_confirmation
User.any_instance.stubs(:mobile_phone).returns('7894561230')
User.any_instance.expects(:authenticate_otp).returns(true)

post :confirm, code: '12345'
if Rails.version < '5.0'
post :confirm, code: '12345', format: 'js'
else
post :confirm, params: { code: '12345' }, format: 'js'
end

@user.reload

Expand All @@ -48,7 +60,11 @@ def test_user_phone_confirmation_with_wrong_code
User.any_instance.expects(:mobile_phone).returns('7894561230')
User.any_instance.expects(:authenticate_otp).returns(false)

post :confirm, code: '12345'
if Rails.version < '5.0'
post :confirm, code: '12345', format: 'js'
else
post :confirm, params: { code: '12345' }, format: 'js'
end

@user.reload

Expand Down
Loading

0 comments on commit 902c913

Please sign in to comment.