Update controller auth

app/controllers/users_controller.rb

@@ -73,7 +73,11 @@ private
   end
 
   def user_params
+    if @user == current_user
       params.require(:user).permit(:email, :name, :password, :password_confirmation, :role)
+    else
+      params.require(:user).permit(:email, :name, :role)
+    end
   end
 
   def find_resource
@@ -81,6 +85,8 @@ private
   end
 
   def authenticate_scope!
-    render_not_found if current_user != @user
+    render_not_found and return unless current_user.organisation == @user.organisation
+    render_not_found and return if action_name == "edit_password" && current_user != @user
+    render_not_found and return unless current_user.role == "data_coordinator" || current_user == @user
   end
 end

app/views/users/edit.html.erb

@@ -1,4 +1,4 @@
-<% content_for :title, "Change your personal details" %>
+<% content_for :title, current_user == @user ? "Change your personal details" : "Change #{@user.name}'s personal details" %>
 
 <% content_for :before_content do %>
   <%= govuk_back_link(

app/views/users/show.html.erb

@@ -1,4 +1,4 @@
-<% content_for :title, "Your account" %>
+<% content_for :title, current_user == @user ? "Your account" : "#{@user.name}'s account" %>
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">

spec/requests/users_controller_spec.rb

@@ -2,7 +2,7 @@ require "rails_helper"
 
 RSpec.describe UsersController, type: :request do
   let(:user) { FactoryBot.create(:user) }
-  let(:unauthorised_user) { FactoryBot.create(:user) }
+  let(:other_user) { FactoryBot.create(:user) }
   let(:headers) { { "Accept" => "text/html" } }
   let(:page) { Capybara::Node::Simple.new(response.body) }
   let(:new_value) { "new test name" }
@@ -109,6 +109,7 @@ RSpec.describe UsersController, type: :request do
     end
   end
 
+  context "when user is signed in as a data provider" do
     describe "#show" do
       context "when the current user matches the user ID" do
         before do
@@ -124,7 +125,7 @@ RSpec.describe UsersController, type: :request do
       context "when the current user does not matches the user ID" do
         before do
           sign_in user
-        get "/users/#{unauthorised_user.id}", headers: headers, params: {}
+          get "/users/#{other_user.id}", headers: headers, params: {}
         end
 
         it "returns not found 404" do
@@ -152,7 +153,7 @@ RSpec.describe UsersController, type: :request do
       context "when the current user does not matches the user ID" do
         before do
           sign_in user
-        get "/users/#{unauthorised_user.id}/edit", headers: headers, params: {}
+          get "/users/#{other_user.id}/edit", headers: headers, params: {}
         end
 
         it "returns not found 404" do
@@ -180,7 +181,7 @@ RSpec.describe UsersController, type: :request do
       context "when the current user does not matches the user ID" do
         before do
           sign_in user
-        get "/users/#{unauthorised_user.id}/edit", headers: headers, params: {}
+          get "/users/#{other_user.id}/edit", headers: headers, params: {}
         end
 
         it "returns not found 404" do
@@ -223,11 +224,11 @@ RSpec.describe UsersController, type: :request do
       end
 
       context "when the current user does not matches the user ID" do
-      let(:params) { { id: unauthorised_user.id, user: { name: new_value } } }
+        let(:params) { { id: other_user.id, user: { name: new_value } } }
 
         before do
           sign_in user
-        patch "/users/#{unauthorised_user.id}", headers: headers, params: params
+          patch "/users/#{other_user.id}", headers: headers, params: params
         end
 
         it "returns not found 404" do
@@ -253,6 +254,224 @@ RSpec.describe UsersController, type: :request do
         end
       end
     end
+  end
+
+  context "when user is signed in as a data coordinator" do
+    let(:user) { FactoryBot.create(:user, :data_coordinator) }
+    let(:other_user) { FactoryBot.create(:user, organisation: user.organisation) }
+
+    describe "#show" do
+      context "when the current user matches the user ID" do
+        before do
+          sign_in user
+          get "/users/#{user.id}", headers: headers, params: {}
+        end
+
+        it "show the user details" do
+          expect(page).to have_content("Your account")
+        end
+      end
+
+      context "when the current user does not matches the user ID" do
+        before do
+          sign_in user
+          get "/users/#{other_user.id}", headers: headers, params: {}
+        end
+
+        context "when the user is part of the same organisation as the current user" do
+          it "returns 200" do
+            expect(response).to have_http_status(:ok)
+          end
+
+          it "shows the user details page" do
+            expect(page).to have_content("#{other_user.name}'s account")
+          end
+        end
+
+        context "when the user is not part of the same organisation as the current user" do
+          let(:other_user) { FactoryBot.create(:user) }
+
+          it "returns not found 404" do
+            expect(response).to have_http_status(:not_found)
+          end
+
+          it "shows the 404 view" do
+            expect(page).to have_content("Page not found")
+          end
+        end
+      end
+    end
+
+    describe "#edit" do
+      context "when the current user matches the user ID" do
+        before do
+          sign_in user
+          get "/users/#{user.id}/edit", headers: headers, params: {}
+        end
+
+        it "show the edit personal details page" do
+          expect(page).to have_content("Change your personal details")
+        end
+      end
+
+      context "when the current user does not matches the user ID" do
+        before do
+          sign_in user
+          get "/users/#{other_user.id}/edit", headers: headers, params: {}
+        end
+
+        context "when the user is part of the same organisation as the current user" do
+          it "returns 200" do
+            expect(response).to have_http_status(:ok)
+          end
+
+          it "shows the user details page" do
+            expect(page).to have_content("Change #{other_user.name}'s personal details")
+          end
+        end
+
+        context "when the user is not part of the same organisation as the current user" do
+          let(:other_user) { FactoryBot.create(:user) }
+
+          it "returns not found 404" do
+            expect(response).to have_http_status(:not_found)
+          end
+        end
+      end
+    end
+
+    describe "#edit_password" do
+      context "when the current user matches the user ID" do
+        before do
+          sign_in user
+          get "/users/#{user.id}/password/edit", headers: headers, params: {}
+        end
+
+        it "shows the edit password page" do
+          expect(page).to have_content("Change your password")
+        end
+
+        it "shows the password requirements hint" do
+          expect(page).to have_css("#user-password-hint")
+        end
+      end
+
+      context "when the current user does not matches the user ID" do
+        before do
+          sign_in user
+          get "/users/#{other_user.id}/password/edit", headers: headers, params: {}
+        end
+
+        it "returns not found 404" do
+          expect(response).to have_http_status(:not_found)
+        end
+      end
+    end
+
+    describe "#update" do
+      context "when the current user matches the user ID" do
+        before do
+          sign_in user
+          patch "/users/#{user.id}", headers: headers, params: params
+        end
+
+        it "updates the user" do
+          user.reload
+          expect(user.name).to eq(new_value)
+        end
+
+        it "tracks who updated the record" do
+          user.reload
+          whodunnit_actor = user.versions.last.actor
+          expect(whodunnit_actor).to be_a(User)
+          expect(whodunnit_actor.id).to eq(user.id)
+        end
+
+        context "when we update the user password" do
+          let(:params) do
+            {
+              id: user.id, user: { password: new_value, password_confirmation: "something_else" }
+            }
+          end
+
+          before do
+            sign_in user
+            patch "/users/#{user.id}", headers: headers, params: params
+          end
+
+          it "shows an error if passwords don't match" do
+            expect(response).to have_http_status(:unprocessable_entity)
+            expect(page).to have_selector("#error-summary-title")
+          end
+        end
+      end
+
+      context "when the current user does not matches the user ID" do
+        before do
+          sign_in user
+        end
+
+        context "when the user is part of the same organisation as the current user" do
+          it "updates the user" do
+            expect { patch "/users/#{other_user.id}", headers: headers, params: params }
+              .to change { other_user.reload.name }.from(other_user.name).to(new_value)
+          end
+
+          it "tracks who updated the record" do
+            expect { patch "/users/#{other_user.id}", headers: headers, params: params }
+              .to change { other_user.reload.versions.last.actor&.id }.from(nil).to(user.id)
+          end
+
+          context "when we try to update the user password" do
+            let(:params) do
+              {
+                id: user.id, user: { password: new_value, password_confirmation: new_value, name: "new name" }
+              }
+            end
+
+            it "does not update the password" do
+              expect { patch "/users/#{other_user.id}", headers: headers, params: params }
+                .not_to change(other_user, :encrypted_password)
+            end
+
+            it "does update other values" do
+              expect { patch "/users/#{other_user.id}", headers: headers, params: params }
+                .to change { other_user.reload.name }.from("Danny Rojas").to("new name")
+            end
+          end
+        end
+
+        context "when the current user does not matches the user ID" do
+          context "when the user is not part of the same organisation as the current user" do
+            let(:other_user) { FactoryBot.create(:user) }
+            let(:params) { { id: other_user.id, user: { name: new_value } } }
+
+            before do
+              sign_in user
+              patch "/users/#{other_user.id}", headers: headers, params: params
+            end
+
+            it "returns not found 404" do
+              expect(response).to have_http_status(:not_found)
+            end
+          end
+        end
+      end
+
+      context "when the update fails to persist" do
+        before do
+          sign_in user
+          allow(User).to receive(:find_by).and_return(user)
+          allow(user).to receive(:update).and_return(false)
+          patch "/users/#{user.id}", headers: headers, params: params
+        end
+
+        it "show an error" do
+          expect(response).to have_http_status(:unprocessable_entity)
+        end
+      end
+    end
+  end
 
   describe "#create" do
     let(:params) do