From 62a4a5f2030f6943d86cc588277ba46919abb236 Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Tue, 30 Jun 2020 14:16:00 +0200 Subject: [PATCH] Add 2fa login --- app/controllers/users/sessions_controller.rb | 64 ++++++++--- app/models/user.rb | 7 ++ app/views/users/sessions/new.html.erb | 2 +- .../users/sessions/two_factor_auth.html.erb | 19 ++++ config/locales/en.yml | 6 + config/routes.rb | 4 +- .../users/sessions_controller_spec.rb | 107 ++++++++++++++++++ spec/models/user_spec.rb | 21 ++++ 8 files changed, 212 insertions(+), 18 deletions(-) create mode 100644 app/views/users/sessions/two_factor_auth.html.erb create mode 100644 spec/controllers/users/sessions_controller_spec.rb diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index a52c97e8e..50d7c7258 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -5,6 +5,7 @@ class Users::SessionsController < Devise::SessionsController # before_filter :configure_sign_in_params, only: [:create] after_action :after_sign_in, only: :create + prepend_before_action :redirect_2fa, only: :create rescue_from ActionController::InvalidAuthenticityToken do redirect_to new_user_session_path @@ -24,21 +25,7 @@ class Users::SessionsController < Devise::SessionsController def create super - # Schedule templates creation for user - TemplatesService.new.schedule_creation_for_user(current_user) - - # Schedule demo project creation for user - current_user.created_teams.each do |team| - FirstTimeDataGenerator.delay( - queue: :new_demo_project, - priority: 10 - ).seed_demo_data_with_id(current_user.id, team.id) - end - rescue StandardError => e - Rails.logger.fatal( - "User ID #{current_user.id}: Error creating inital projects on sign_in: "\ - "#{e.message}" - ) + generate_demo_project end # DELETE /resource/sign_out @@ -75,6 +62,27 @@ class Users::SessionsController < Devise::SessionsController flash[:system_notification_modal] = true end + def authenticate_with_two_factor + user = User.find_by(id: session[:otp_user_id]) + + unless user + flash[:alert] = t('devise.sessions.2fa.no_user_error') + redirect_to root_path && return + end + + if user.valid_otp?(params[:otp]) + session.delete(:otp_user_id) + + sign_in(user) + generate_demo_project + flash[:notice] = t('devise.sessions.signed_in') + redirect_to root_path + else + flash.now[:alert] = t('devise.sessions.2fa.error_message') + render :two_factor_auth + end + end + protected # If you have extra params to permit, append them to the sanitizer. @@ -84,6 +92,32 @@ class Users::SessionsController < Devise::SessionsController private + def redirect_2fa + user = User.find_by(email: params[:user][:email]) + + return unless user&.valid_password?(params[:user][:password]) + + if user&.two_factor_auth_enabled? + session[:otp_user_id] = user.id + render :two_factor_auth + end + end + + def generate_demo_project + # Schedule templates creation for user + TemplatesService.new.schedule_creation_for_user(current_user) + + # Schedule demo project creation for user + current_user.created_teams.each do |team| + FirstTimeDataGenerator.delay( + queue: :new_demo_project, + priority: 10 + ).seed_demo_data_with_id(current_user.id, team.id) + end + rescue StandardError => e + Rails.logger.fatal("User ID #{current_user.id}: Error creating inital projects on sign_in: #{e.message}") + end + def session_layout if @simple_sign_in 'sign_in_halt' diff --git a/app/models/user.rb b/app/models/user.rb index c3614802d..5b94c7b7f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -623,6 +623,13 @@ class User < ApplicationRecord avatar.blob&.filename&.sanitized end + def valid_otp?(otp) + raise StandardError, 'Missing otp_secret' unless otp_secret + + totp = ROTP::TOTP.new(otp_secret, issuer: 'sciNote') + totp.verify(otp, drift_behind: 10) + end + protected def confirmation_required? diff --git a/app/views/users/sessions/new.html.erb b/app/views/users/sessions/new.html.erb index 9d162a5a3..a12098fd2 100644 --- a/app/views/users/sessions/new.html.erb +++ b/app/views/users/sessions/new.html.erb @@ -43,4 +43,4 @@ <%= render partial: "users/shared/links", locals: {linkedin_skip: true} unless @simple_sign_in %> - \ No newline at end of file + diff --git a/app/views/users/sessions/two_factor_auth.html.erb b/app/views/users/sessions/two_factor_auth.html.erb new file mode 100644 index 000000000..ec1de0fa4 --- /dev/null +++ b/app/views/users/sessions/two_factor_auth.html.erb @@ -0,0 +1,19 @@ +<% provide(:head_title, t("devise.sessions.new.head_title")) %> +<% content_for(:body_class, 'sign-in-layout') %> +
+ +
diff --git a/config/locales/en.yml b/config/locales/en.yml index 81a487b8b..8b1cfce2a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -33,6 +33,12 @@ en: password_placeholder: "Enter password" remember_me: "Remember me" submit: "Log in" + 2fa: + title: "Two factor check" + field: "One time password" + placeholder: "Enter code" + error_message: "One Time Password is not correct." + no_user_error: "Cannot find user!" create: team_name: "%{user}'s projects" auth_token_create: diff --git a/config/routes.rb b/config/routes.rb index 206b1649d..c5c38009e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -695,8 +695,8 @@ Rails.application.routes.draw do get 'avatar/:id/:style' => 'users/registrations#avatar', as: 'avatar' get 'users/auth_token_sign_in' => 'users/sessions#auth_token_create' get 'users/sign_up_provider' => 'users/registrations#new_with_provider' - post 'users/complete_sign_up_provider' => - 'users/registrations#create_with_provider' + post 'users/authenticate_with_two_factor' => 'users/sessions#authenticate_with_two_factor' + post 'users/complete_sign_up_provider' => 'users/registrations#create_with_provider' end namespace :api, defaults: { format: 'json' } do diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb new file mode 100644 index 000000000..718939da1 --- /dev/null +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Users::SessionsController, type: :controller do + describe 'POST #create' do + before do + @request.env['devise.mapping'] = Devise.mappings[:user] + end + + let(:user) { create :user } + let(:password) { 'asdf1243' } + let(:params) do + { user: { + email: user.email, + password: password + } } + end + + let(:action) do + post :create, params: params + end + + context 'when have invalid email or password' do + let(:password) { '123' } + + it 'returns error message' do + action + + expect(flash[:alert]).to eq('Invalid Email or password.') + end + + it 'does not set current user' do + expect { action }.not_to(change { subject.current_user }) + end + end + + context 'when have valid email and password' do + context 'when user has 2FA disabled' do + it 'returns successfully log in' do + action + + expect(flash[:notice]).to eq('Logged in successfully.') + end + + it 'sets current user' do + expect { action }.to(change { subject.current_user }.from(nil).to(User)) + end + end + + context 'when user has 2FA enabled' do + it 'renders 2FA page' do + user.two_factor_auth_enabled = true + user.save! + + expect(action).to render_template('users/sessions/two_factor_auth') + end + end + end + end + + describe 'POST #authenticate_with_two_factor' do + before do + @request.env['devise.mapping'] = Devise.mappings[:user] + end + + let(:user) { create :user } + let(:params) { { otp: '123123' } } + let(:otp_user_id) { user.id } + let(:action) do + post :authenticate_with_two_factor, params: params, session: { otp_user_id: otp_user_id } + end + + context 'when have valid otp' do + it 'sets current user' do + allow_any_instance_of(User).to receive(:valid_otp?).and_return(true) + + expect { action }.to(change { subject.current_user }.from(nil).to(User)) + end + end + + context 'when have invalid valid otp' do + it 'returns error message' do + allow_any_instance_of(User).to receive(:valid_otp?).and_return(nil) + action + + expect(flash[:alert]).to eq('One Time Password is not correct.') + end + + it 'does not set current user' do + allow_any_instance_of(User).to receive(:valid_otp?).and_return(nil) + + expect { action }.not_to(change { subject.current_user }) + end + end + + context 'when user is not found' do + let(:otp_user_id) { -1 } + + it 'returns error message' do + action + + expect(flash[:alert]).to eq('Cannot find user!') + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f9af6ec2a..ae6bbf1c4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -369,4 +369,25 @@ describe User, type: :model do end end end + + describe 'valid_otp?' do + let(:user) { create :user } + before do + allow_any_instance_of(ROTP::TOTP).to receive(:verify).and_return(nil) + end + + context 'when user has set otp_secret' do + it 'returns nil' do + expect(user.valid_otp?('someString')).to be_nil + end + end + + context 'when user does not have otp_secret' do + it 'raises an error' do + user.update_column(:otp_secret, nil) + + expect { user.valid_otp?('someString') }.to raise_error(StandardError, 'Missing otp_secret') + end + end + end end