Merge pull request #2545 from alphagov/split-out-edit-user-2sv-pages-…

…from-user-edit-page

Add separate page for resetting another user's 2SV

app/controllers/users/two_step_verification_resets_controller.rb

@@ -0,0 +1,31 @@
+class Users::TwoStepVerificationResetsController < ApplicationController
+  layout "admin_layout"
+
+  before_action :authenticate_user!
+  before_action :load_user
+  before_action :authorize_user
+  before_action :redirect_to_account_page_if_acting_on_own_user, only: %i[edit]
+
+  def edit; end
+
+  def update
+    @user.reset_2sv!(current_user)
+    UserMailer.two_step_reset(@user).deliver_later
+
+    redirect_to edit_user_path(@user), notice: "Reset 2-step verification for #{@user.email}"
+  end
+
+private
+
+  def load_user
+    @user = User.find(params[:user_id])
+  end
+
+  def authorize_user
+    authorize(@user, :reset_2sv?)
+  end
+
+  def redirect_to_account_page_if_acting_on_own_user
+    redirect_to two_step_verification_path if current_user == @user
+  end
+end

app/controllers/users_controller.rb

@@ -57,13 +57,6 @@ def event_logs
     @logs = @user.event_logs.page(params[:page]).per(100) if @user
   end
 
-  def reset_two_step_verification
-    @user.reset_2sv!(current_user)
-    UserMailer.two_step_reset(@user).deliver_later
-
-    redirect_to users_path, notice: "Reset 2-step verification for #{@user.email}"
-  end
-
   def require_2sv; end
 
 private

app/models/user.rb

@@ -393,7 +393,7 @@ def exempt_from_2sv(reason, initiating_user, expiry_date)
     end
   end
 
-  def reset_2sv!(initiating_superadmin)
+  def reset_2sv!(initiator)
     transaction do
       self.otp_secret_key = nil
       self.require_2sv = true
@@ -402,7 +402,7 @@ def reset_2sv!(initiating_superadmin)
       EventLog.record_event(
         self,
         EventLog::TWO_STEP_RESET,
-        initiator: initiating_superadmin,
+        initiator:,
       )
     end
   end

app/views/users/_form_fields.html.erb

@@ -30,58 +30,54 @@
   <% end %>
 </p>
 
-<% if policy(@user).mandate_2sv? %>
-  <dl>
-    <dt>Account security</dt>
-    <dd>
-      <% if @user.exempt_from_2sv? %>
-        <p>
-          The user has been made exempt from 2-step verification for the following reason: <%= @user.reason_for_2sv_exemption %>
+<dl>
+  <dt>Account security</dt>
+  <dd>
+    <% if @user.exempt_from_2sv? %>
+      <p>
+        The user has been made exempt from 2-step verification for the following reason: <%= @user.reason_for_2sv_exemption %>
         <% if policy(@user).exempt_from_two_step_verification? %>
           <br>
           <%= link_to 'Edit reason or expiry date for 2-step verification exemption', edit_two_step_verification_exemption_path(@user) %>
         <% end %>
-        </p>
-      <% elsif f.object.has_2sv? %>
-        <p>2-step verification enabled</p>
-        <p>
-          <%= link_to 'Reset 2-step verification',
-            reset_two_step_verification_user_path(@user),
-            data: { confirm: 'Are you sure?' },
-            method: :patch
-          %><br/>
-          Allows user to sign in without a verification code.<br/>
-          User will be prompted to set up 2-step verification again the next time they sign in.
-        </p>
-      <% elsif f.object.require_2sv? %>
-        <p>2-step verification required but not set up</p>
-      <% else %>
-        <p>2-step verification not set up</p>
-      <% end %>
+      </p>
+    <% elsif @user.has_2sv? %>
+      <p>2-step verification enabled</p>
+      <p>
+        <%= link_to 'Reset 2-step verification', edit_user_two_step_verification_reset_path(@user) %>
+      </p>
+    <% elsif @user.require_2sv? %>
+      <p>2-step verification required but not set up</p>
+    <% else %>
+      <p>2-step verification not set up</p>
+    <% end %>
 
-      <% unless @user.require_2sv? %>
+    <% unless @user.require_2sv? %>
+      <% if policy(@user).mandate_2sv? %>
         <p class="checkbox">
           <%= f.label :require_2sv do %>
             <%= f.check_box :require_2sv %> Mandate 2-step verification for this user <%= "(this will remove their exemption)" if @user.exempt_from_2sv? %>
           <% end %>
-        <br/>
-        User will be prompted to set up 2-step verification again the next time they sign in.</p>
+          <br/>
+          User will be prompted to set up 2-step verification again the next time they sign in.
+        </p>
       <% end %>
-      <% if policy(@user).exempt_from_two_step_verification? && @user.reason_for_2sv_exemption.nil? %>
-          <p>
-            <%= link_to 'Exempt user from 2-step verification', edit_two_step_verification_exemption_path(@user) %>
-            <br/>
-            Exempt a user from 2-step verification (requires a reason to be supplied).
-          </p>
-      <%end %>
-    </dd>
-  </dl>
-<% end %>
+    <% end %>
+
+    <% if policy(@user).exempt_from_two_step_verification? && !@user.exempt_from_2sv? %>
+        <p>
+          <%= link_to 'Exempt user from 2-step verification', edit_two_step_verification_exemption_path(@user) %>
+          <br/>
+          Exempt a user from 2-step verification (requires a reason to be supplied).
+        </p>
+    <% end %>
+  </dd>
+</dl>
 
 <h2 class="add-vertical-margins"> <%= "Editable " if (current_user.publishing_manager? ) %>Permissions</h2>
-<%= render partial: "shared/user_permissions", locals: { user_object: f.object }%>
+<%= render partial: "shared/user_permissions", locals: { user_object: @user }%>
 
 <% if current_user.publishing_manager? %>
     <h2 class="add-vertical-margins">All Permissions for this user</h2>
-    <%= render partial: "app_permissions", locals: { user_object: f.object }%>
+    <%= render partial: "app_permissions", locals: { user_object: @user }%>
 <% end %>

app/views/users/two_step_verification_resets/edit.html.erb

@@ -0,0 +1,42 @@
+<% content_for :title_caption, "Manage other users" %>
+<% content_for :title, "Reset 2-step verification for #{@user.name}" %>
+
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Dashboard",
+         url: root_path,
+       },
+       {
+         title: "Users",
+         url: users_path,
+       },
+       {
+         title: @user.name,
+         url: edit_user_path(@user),
+       },
+       {
+         title: "Reset 2-step verification",
+       }
+     ]
+   })
+%>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <%= form_for @user, url: user_two_step_verification_reset_path(@user) do %>
+      <%= render "govuk_publishing_components/components/hint", {
+        text: "Allows user to sign in without a verification code. User will be prompted to set up 2-step verification again the next time they sign in."
+      } %>
+      <div class="govuk-button-group">
+        <%= render "govuk_publishing_components/components/button", {
+          text: "Reset 2-step verification",
+          destructive: true,
+        } %>
+        <%= link_to "Cancel", edit_user_path(@user), class: "govuk-link govuk-link--no-visited-state" %>
+      </div>
+    <% end %>
+  </div>
+</div>

config/routes.rb

@@ -40,7 +40,6 @@
     member do
       post :unlock
       get :event_logs
-      patch :reset_two_step_verification
       get :require_2sv
     end
     resource :name, only: %i[edit update], controller: "users/names"
@@ -50,6 +49,7 @@
     end
     resource :role, only: %i[edit update], controller: "users/roles"
     resource :organisation, only: %i[edit update], controller: "users/organisations"
+    resource :two_step_verification_reset, only: %i[edit update], controller: "users/two_step_verification_resets"
   end
   get "user", to: "oauth_users#show"
 

test/controllers/users/two_step_verification_resets_controller_test.rb

@@ -0,0 +1,173 @@
+require "test_helper"
+
+class Users::TwoStepVerificationResetsControllerTest < ActionController::TestCase
+  include ActiveJob::TestHelper
+
+  context "GET edit" do
+    context "signed in as Admin user" do
+      setup do
+        @admin = create(:admin_user)
+        sign_in(@admin)
+      end
+
+      should "display form with submit button & cancel link" do
+        user = create(:two_step_enabled_user)
+
+        get :edit, params: { user_id: user }
+
+        assert_template :edit
+        assert_select "form[action='#{user_two_step_verification_reset_path(user)}']" do
+          assert_select "button[type='submit']", text: "Reset 2-step verification"
+          assert_select "a[href='#{edit_user_path(user)}']", text: "Cancel"
+        end
+      end
+
+      should "authorize access if UserPolicy#reset_2sv? returns true" do
+        user = create(:two_step_enabled_user)
+
+        user_policy = stub_everything("user-policy", reset_2sv?: true)
+        UserPolicy.stubs(:new).returns(user_policy)
+
+        get :edit, params: { user_id: user }
+
+        assert_template :edit
+      end
+
+      should "not authorize access if UserPolicy#reset_2sv? returns false" do
+        user = create(:two_step_enabled_user)
+
+        user_policy = stub_everything("user-policy", reset_2sv?: false)
+        UserPolicy.stubs(:new).returns(user_policy)
+
+        get :edit, params: { user_id: user }
+
+        assert_not_authorised
+      end
+
+      should "redirect to account change 2-step verification phone page if admin is acting on their own user" do
+        get :edit, params: { user_id: @admin }
+
+        assert_redirected_to two_step_verification_path
+      end
+    end
+
+    context "signed in as Normal user" do
+      setup do
+        sign_in(create(:user))
+      end
+
+      should "not be authorized" do
+        user = create(:user)
+
+        get :edit, params: { user_id: user }
+
+        assert_not_authorised
+      end
+    end
+
+    context "not signed in" do
+      should "not be allowed access" do
+        user = create(:user)
+
+        get :edit, params: { user_id: user }
+
+        assert_not_authenticated
+      end
+    end
+  end
+
+  context "PUT update" do
+    context "signed in as Admin user" do
+      setup do
+        @admin = create(:admin_user)
+        sign_in(@admin)
+      end
+
+      should "reset 2SV for user" do
+        user = create(:two_step_enabled_user)
+
+        put :update, params: { user_id: user }
+
+        user.reload
+        assert user.otp_secret_key.blank?
+        assert user.require_2sv?
+      end
+
+      should "record account updated event" do
+        user = create(:two_step_enabled_user)
+
+        EventLog.expects(:record_event).with(user, EventLog::TWO_STEP_RESET, initiator: @admin)
+
+        put :update, params: { user_id: user }
+      end
+
+      should "should send email notifying user that their 2SV has been reset" do
+        user = create(:two_step_enabled_user)
+
+        perform_enqueued_jobs do
+          put :update, params: { user_id: user }
+        end
+
+        email = ActionMailer::Base.deliveries.last
+        assert email.present?
+        assert_equal "2-step verification has been reset", email.subject
+      end
+
+      should "redirect to user page and display success notice" do
+        user = create(:two_step_enabled_user, email: "user@gov.uk")
+
+        put :update, params: { user_id: user }
+
+        assert_redirected_to edit_user_path(user)
+        assert_equal "Reset 2-step verification for user@gov.uk", flash[:notice]
+      end
+
+      should "reset 2SV for user if UserPolicy#reset_2sv? returns true" do
+        user = create(:two_step_enabled_user)
+
+        user_policy = stub_everything("user-policy", reset_2sv?: true)
+        UserPolicy.stubs(:new).returns(user_policy)
+
+        put :update, params: { user_id: user }
+
+        assert user.reload.otp_secret_key.blank?
+      end
+
+      should "not reset 2SV for user if UserPolicy#reset_2sv? returns false" do
+        user = create(:two_step_enabled_user)
+
+        user_policy = stub_everything("user-policy", reset_2sv?: false)
+        UserPolicy.stubs(:new).returns(user_policy)
+
+        put :update, params: { user_id: user }
+
+        assert user.reload.otp_secret_key.present?
+        assert_not_authorised
+      end
+    end
+
+    context "signed in as Normal user" do
+      setup do
+        sign_in(create(:user))
+      end
+
+      should "not be authorized" do
+        user = create(:two_step_enabled_user)
+
+        put :update, params: { user_id: user }
+
+        assert_not_authorised
+      end
+    end
+
+    context "not signed in" do
+      should "not be allowed access" do
+        user = create(:two_step_enabled_user)
+
+        put :update, params: { user_id: user }
+
+        assert_not_authenticated
+      end
+    end
+  end
+end