Commit 4a7db90

mo <mo@mokhan.ca>
2018-12-17 23:15:08
require mfa code to disable mfa
1 parent 04ba597
Changed files (6)
app
config
locales
spec
requests
app/controllers/my/mfas_controller.rb
@@ -27,8 +27,11 @@ module My
     end
 
     def destroy
-      current_user.mfa.disable!
-      redirect_to my_dashboard_path, notice: 'MFA has been disabled'
+      if current_user.mfa.disable!(params[:user][:code])
+        redirect_to my_dashboard_path, notice: 'MFA has been disabled'
+      else
+        redirect_to edit_my_mfa_path, error: "MFA code is incorrect"
+      end
     end
   end
 end
app/models/mfa.rb
@@ -19,7 +19,8 @@ class Mfa
     user.mfa_secret = ::ROTP::Base32.random_base32
   end
 
-  def disable!
+  def disable!(entered_code)
+    return false unless authenticate(entered_code)
     user.update!(mfa_secret: nil)
   end
 
app/views/my/mfas/edit.html.erb
@@ -1,25 +1,25 @@
+<% content_for :page_title, t(".title") %>
+
 <div class="container">
   <div class="columns is-centered">
-    <div class="column is-two-thirds">
+    <div class="column is-half">
       <h1 class="title"><%= t('.title') %></h1>
-
-      <div data-controller="mfa--setup">
-        <canvas id="canvas" data-target="mfa--setup.canvas"></canvas>
-        <p><%= t('.secret') %> <%= current_user.mfa.secret %></p>
-        <p><%= t('.provisioning_uri') %> <%= current_user.mfa.provisioning_uri %></p>
-
-        <%= form_for current_user, url: my_mfa_path, method: :delete do |form| %>
-          <%= form.hidden_field :mfa_secret, data: { target: 'mfa--setup.secret' } %>
-          <div class="field is-grouped is-grouped-right">
-            <div class="control">
-              <%= form.submit "Disable", class: 'button is-danger', data: { disable_with: 'Saving…' } %>
-            </div>
-            <div class="control">
-              <%= link_to "Cancel", my_dashboard_path, class: 'button is-text' %>
-            </div>
+      <p class="content">MFA gives you a second line of defense against unauthorized attempts to access your account.</p>
+      <%= form_for current_user, url: my_mfa_path, method: :delete do |form| %>
+        <div class="field">
+          <div class="control">
+            <%= form.number_field :code, value: '', placeholder: 'code', class: 'input', required: :required, data: { controller: 'input' } %>
+          </div>
+        </div>
+        <div class="field is-grouped is-grouped-right">
+          <div class="control">
+            <%= form.submit t(".disable"), class: 'button is-danger', data: { disable_with: t('saving') } %>
+          </div>
+          <div class="control">
+            <%= link_to t("cancel"), my_dashboard_path, class: 'button' %>
           </div>
-        <% end %>
-      </div>
+        </div>
+      <% end %>
     </div>
   </div>
 </div>
app/views/my/mfas/new.html.erb
@@ -62,10 +62,10 @@
         <%= form.hidden_field :mfa_secret, data: { target: 'mfa--setup.secret' } %>
         <div class="field is-grouped is-grouped-right">
           <div class="control">
-            <%= form.submit t(".enable"), id: 'enable-button', class: 'button is-primary', data: { disable_with: t('.saving') } %>
+            <%= form.submit t(".enable"), class: 'button is-primary', data: { disable_with: t('saving') } %>
           </div>
           <div class="control">
-            <%= link_to t(".cancel"), my_dashboard_path, class: 'button' %>
+            <%= link_to t("cancel"), my_dashboard_path, class: 'button' %>
           </div>
         </div>
       <% end %>
config/locales/en.yml
@@ -37,7 +37,6 @@ en:
     application:
       title: Proof
   loading: Loading…
-  saving: Saving…
   mfas:
     new:
       login: Login
@@ -68,12 +67,16 @@ en:
         title: Dashboard
     mfas:
       edit:
+        cancel: Cancel
+        disable: Disable
         provisioning_uri: Provisioning uri
+        saving: Saving
         secret: Secret
         title: Multi-Factor Authentication (MFA)
       new:
         cancel: Cancel
         enable: Enable
+        saving: Saving
         test: Test
         title: Multi Factor Authentication - Setup
       test:
@@ -101,6 +104,7 @@ en:
   responses:
     response:
       title: Sending Response to Service Provider
+  saving: Saving…
   scim:
     errors:
       user:
spec/requests/my/mfas_spec.rb
@@ -73,15 +73,27 @@ RSpec.describe '/my/mfa' do
     end
 
     describe "DELETE /my/mfa" do
-      context "when mfa is enabled" do
+      context "when mfa is enabled and the code is correct" do
         let(:current_user) { create(:user, :mfa_configured) }
+        let(:current_code) { current_user.mfa.current_totp }
 
-        before { delete '/my/mfa' }
+        before { delete '/my/mfa', params: { user: { code: current_code } } }
 
         specify { expect(current_user.reload.mfa_secret).to be_nil }
         specify { expect(response).to redirect_to(my_dashboard_path) }
         specify { expect(flash[:notice]).to include("MFA has been disabled") }
       end
+
+      context "when mfa is enabled and the code is incorrect" do
+        let(:current_user) { create(:user, :mfa_configured) }
+        let!(:original_secret) { current_user.mfa_secret }
+
+        before { delete '/my/mfa', params: { user: { code: 'incorrect' } } }
+
+        specify { expect(current_user.reload.mfa_secret).to eql(original_secret) }
+        specify { expect(response).to redirect_to(edit_my_mfa_path) }
+        specify { expect(flash[:error]).to include("MFA code is incorrect") }
+      end
     end
   end