From c5929544002fe0f68c9bdc167f4a163e3fa2641e Mon Sep 17 00:00:00 2001
From: aignatov-bio <aignatov@biosistemika.com>
Date: Mon, 13 Jul 2020 16:05:23 +0200
Subject: [PATCH] Add UX

---
 app/controllers/users/sessions_controller.rb    |  6 ++++--
 app/models/user.rb                              |  2 +-
 .../users/sessions/two_factor_auth.html.erb     | 17 +++++++++--------
 .../users/sessions/two_factor_recovery.html.erb | 13 +++++++------
 config/locales/en.yml                           | 16 ++++++++++++----
 5 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb
index c11e25e15..ca575eddb 100644
--- a/app/controllers/users/sessions_controller.rb
+++ b/app/controllers/users/sessions_controller.rb
@@ -102,10 +102,12 @@ class Users::SessionsController < Devise::SessionsController
       sign_in(user)
       generate_demo_project
       flash[:notice] = t('devise.sessions.signed_in')
+      redirect_to root_path
     else
-      flash.now[:alert] = t('Not correct recovery code')
+      flash[:alert] = t("devise.sessions.2fa_recovery.not_correct_code")
+      redirect_to new_user_session_path
     end
-    redirect_to root_path
+
   end
 
   protected
diff --git a/app/models/user.rb b/app/models/user.rb
index 268ac7e99..cc0dd5cc4 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -676,7 +676,7 @@ class User < ApplicationRecord
 
     otp_recovery_codes.each do |recovery_code|
       if Devise::Encryptor.compare(self.class, recovery_code, code)
-        update(otp_recovery_codes: otp_recovery_codes.reject { |i| i == recovery_code })
+        update!(otp_recovery_codes: otp_recovery_codes.reject { |i| i == recovery_code })
         return true
       end
     end
diff --git a/app/views/users/sessions/two_factor_auth.html.erb b/app/views/users/sessions/two_factor_auth.html.erb
index 38dfdec22..c2db9afa5 100644
--- a/app/views/users/sessions/two_factor_auth.html.erb
+++ b/app/views/users/sessions/two_factor_auth.html.erb
@@ -3,18 +3,19 @@
 <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>
+      <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">
+        <p><%= t "devise.sessions.2fa.description" %></p>
+        <p 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>
+          <%= text_field_tag(:otp, '', { class: "form-control sci-input-field" })%>
+        </p>
 
-        <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>
+        <p class="actions">
+          <%= button_tag t("devise.sessions.2fa.enter"), type: :submit, class: "btn btn-primary" %>
+        </p>
 
-        <%= link_to 'I have a bypass code', users_two_factor_recovery_path %>
+        <%= link_to t("devise.sessions.2fa.bypass_code_link"), users_two_factor_recovery_path %>
     <% end %>
     </div>
   </div>
diff --git a/app/views/users/sessions/two_factor_recovery.html.erb b/app/views/users/sessions/two_factor_recovery.html.erb
index d614d621f..3b7f6eaa4 100644
--- a/app/views/users/sessions/two_factor_recovery.html.erb
+++ b/app/views/users/sessions/two_factor_recovery.html.erb
@@ -3,15 +3,16 @@
 <div class="sign-in-container">
   <div class="sign-in-form-wrapper">
     <div class="center-block center-block-narrow">
-      <h1 class="log-in-title">2FA Bypass</h1>
+      <h1 class="log-in-title"><%= t "devise.sessions.2fa_recovery.title" %></h1>
       <%= form_with url: users_authenticate_with_recovery_code_path, local: true  do %>
-        <div class="input-group sci-input-container">
-          <%= label :recovery_code, 'Bypass code' %>
+        <p><%= t "devise.sessions.2fa_recovery.description" %></p>
+        <p class="input-group sci-input-container">
+          <%= label :recovery_code, t("devise.sessions.2fa_recovery.bypass_code") %>
           <%= text_field_tag(:recovery_code, '', { class: "form-control sci-input-field" })%>
-        </div>
+        </p>
 
-        <div class="actions" style="margin-top: 10px; margin-bottom: 10px;">
-          <%= button_tag 'Enter', type: :submit, class: "btn btn-primary" %>
+        <div class="actions">
+          <%= button_tag t("devise.sessions.2fa_recovery.enter"), type: :submit, class: "btn btn-primary" %>
         </div>
     <% end %>
     </div>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index b13149b13..92bb60692 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -34,11 +34,19 @@ en:
         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."
+        title: "Two-factor authentication"
+        description: "Enter the one-time code found in your authenticator app to log in to SciNote."
+        field: "Authenticator code"
+        error_message: "One time code is not correct."
         no_user_error: "Cannot find user!"
+        enter: "Enter"
+        bypass_code_link: "I have a bypass code"
+      2fa_recovery:
+        title: "2FA Bypass"
+        description: "Enter one of the bypass codes provided when you creted 2FA authentication. The code will no longer be valid after use."
+        bypass_code: "Bypass code"
+        enter: "Enter"
+        not_correct_code: "Not correct recovery code"
       create:
         team_name: "%{user}'s projects"
       auth_token_create: