Merge pull request #2696 from urbanrotnik/ur-sci-4753

Add 2FA to login flow [SCI-4753]

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
+    generate_demo_project
-    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
 
   # 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'

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?

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') %>
+<div class="sign-in-container">
+  <div class="sign-in-form-wrapper">
+    <div class="center-block center-block-narrow">
+      <h1 class="log-in-title"><%=t "devise.sessions.2fa.title" %></h1>
+      <%= form_with url: users_authenticate_with_two_factor_url, local: true  do %>
+        <div class="input-group sci-input-container">
+          <%= label :otp, t("devise.sessions.2fa.field") %>
+          <%= text_field_tag(:otp, '', { class: "form-control sci-input-field", placeholder: t("devise.sessions.2fa.placeholder") })%>
+        </div>
+
+        <div class="actions" style="margin-top: 10px; margin-bottom: 10px;">
+          <%= button_tag t("devise.sessions.new.submit"), type: :submit, class: "btn btn-primary log-in-button" %>
+        </div>
+    <% end %>
+    </div>
+  </div>
+</div>

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:

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' =>
+      post 'users/authenticate_with_two_factor' => 'users/sessions#authenticate_with_two_factor'
-           'users/registrations#create_with_provider'
+      post 'users/complete_sign_up_provider' => 'users/registrations#create_with_provider'
     end
 
     namespace :api, defaults: { format: 'json' } do

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

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