diff --git a/Gemfile b/Gemfile index 047d64b3b..7cf883a0b 100644 --- a/Gemfile +++ b/Gemfile @@ -75,6 +75,7 @@ gem 'nokogiri', '~> 1.10.8' # HTML/XML parser gem 'rails_autolink', '~> 1.1', '>= 1.1.6' gem 'rgl' # Graph framework for project diagram calculations gem 'roo', '~> 2.8.2' # Spreadsheet parser +gem 'rotp' gem 'rubyzip' gem 'scenic', '~> 1.4' gem 'sdoc', '~> 1.0', group: :doc diff --git a/Gemfile.lock b/Gemfile.lock index f6c6c236a..0f2a73a0b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -469,6 +469,8 @@ GEM roo (2.8.2) nokogiri (~> 1) rubyzip (>= 1.2.1, < 2.0.0) + rotp (6.0.0) + addressable (~> 2.7) rspec-core (3.8.2) rspec-support (~> 3.8.0) rspec-expectations (3.8.4) @@ -668,6 +670,7 @@ DEPENDENCIES recaptcha rgl roo (~> 2.8.2) + rotp rspec-rails (>= 4.0.0.beta2) rubocop (>= 0.75.0) rubocop-performance diff --git a/app/models/user.rb b/app/models/user.rb index 45907fcf0..b843ea102 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -286,6 +286,8 @@ class User < ApplicationRecord foreign_key: :resource_owner_id, dependent: :delete_all + before_save :ensure_2fa_token, if: ->(user) { user.changed.include?('twofa_enabled') } + before_create :generate_2fa_token before_destroy :destroy_notifications def name @@ -658,4 +660,12 @@ class User < ApplicationRecord def clear_view_cache Rails.cache.delete_matched(%r{^views\/users\/#{id}-}) end + + def generate_2fa_token + self.otp_secret = ROTP::Base32.random + end + + def ensure_2fa_token + generate_2fa_token unless otp_secret + end end diff --git a/db/migrate/20200622140843_add2fa_to_users.rb b/db/migrate/20200622140843_add2fa_to_users.rb new file mode 100644 index 000000000..98d429169 --- /dev/null +++ b/db/migrate/20200622140843_add2fa_to_users.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class Add2faToUsers < ActiveRecord::Migration[6.0] + def change + change_table :users, bulk: true do |t| + t.boolean :twofa_enabled, default: false, null: false + t.string :otp_secret + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 951f4faa2..0126d461d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2647,7 +2647,9 @@ CREATE TABLE public.users ( current_team_id bigint, authentication_token character varying(30), settings jsonb DEFAULT '{}'::jsonb NOT NULL, - variables jsonb DEFAULT '{}'::jsonb NOT NULL + variables jsonb DEFAULT '{}'::jsonb NOT NULL, + twofa_enabled boolean DEFAULT false NOT NULL, + otp_secret character varying ); @@ -7194,6 +7196,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200113143828'), ('20200204100934'), ('20200326114643'), -('20200331183640'); +('20200331183640'), +('20200622140843'); diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8c6ff65ff..a12cf0637 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -331,4 +331,42 @@ describe User, type: :model do describe 'Associations' do it { is_expected.to have_many(:system_notifications) } end + + describe 'Callbacks' do + describe 'after_create' do + it 'sets token' do + user = create :user + + expect(user.otp_secret).to be_kind_of String + end + end + + describe 'before_save' do + let(:user) { create :user } + + context 'when changing twofa_enabled' do + context 'when user does not have otp_secret' do + it 'sets token before save' do + user.update_column(:otp_secret, nil) + + expect { user.update(twofa_enabled: true) }.to(change { user.otp_secret }) + end + end + + context 'when user does have otp_secret' do + it 'does not set new token before save' do + expect { user.update(twofa_enabled: true) }.not_to(change { user.otp_secret }) + end + end + end + + context 'when changing not twofa_enabled and user does have otp_secret' do + it 'does not set token before save' do + user.update_column(:otp_secret, nil) + + expect { user.update(name: 'SomeNewName') }.not_to(change { user.otp_secret }) + end + end + end + end end