extra permissions checks for user updates, controller specs

app/controllers/ui/users_controller.rb

@@ -48,12 +48,22 @@ def create
 
     def edit
       @user = User.friendly.find(params[:id])
+      unless authorize? @user, :update?
+        flash[:alert] = I18n.t(:edit_user_forbidden)
+        redirect_to current_user ? ui_user_path(@user.username) : login_path
+        return
+      end
       @title = I18n.t(:edit_user_title)
     end
 
     def update
       @user = User.friendly.find(params[:id])
-      @user.update(params.require(:user).permit(
+      unless authorize? @user, :update?
+        flash[:alert] = I18n.t(:edit_user_forbidden)
+        redirect_to current_user ? ui_user_path(@user.username) : login_path
+        return
+      end
+      if @user.update(params.require(:user).permit(
         :profile_picture,
         :username,
         :email,
@@ -63,13 +73,11 @@ def update
         :country_code,
         :url
       ))
-      if @user.valid?
-        @user.save
         flash[:success] = I18n.t(:update_user_success)
         redirect_to ui_user_path(@user.username)
       else
         flash[:alert] = I18n.t(:update_user_failure)
-        redirect_to edit_ui_user_path(@user.username)
+        render :new, status: :unprocessable_entity
       end
     end
 
@@ -85,7 +93,7 @@ def delete
 
     def destroy
       @user = User.friendly.find(params[:id])
-      unless authorize? @user
+      unless authorize? @user, :destroy?
         flash[:alert] = I18n.t(:delete_user_forbidden)
         redirect_to current_user ? ui_users_path : login_path
         return

config/locales/controllers/en.yml

@@ -15,6 +15,7 @@ en:
     new_user_success: "Thanks for signing up! You are now logged in."
     new_user_failure: "Some errors prevented us from creating your account. Please check below and try again!"
     edit_user_title: "Edit your profile"
+    edit_user_forbidden: "You are not allowed to edit that user account!"
     update_user_success: "Your profile has been updated!"
     update_user_failure: "Some errors prevented us from updating your profile. Please check below and try again!"
     delete_user_title: "Delete your account"

spec/controllers/ui/users_controller_spec.rb

@@ -14,7 +14,7 @@
 
   describe "show" do
     it "renders the template" do
-      get :show, params: { id: user.id }
+      get :show, params: { id: user.username }
       expect(response).to have_http_status(:success)
       expect(response).to render_template(:show)
     end
@@ -36,7 +36,7 @@
         expect(flash[:alert]).to be_present
       end
     end
-    end
+  end
 
   describe "create" do
     let(:user_params) {
@@ -86,14 +86,97 @@
           expect(flash[:alert]).to be_present
         end
       end
+    end
+  end
+
+
+  describe "edit" do
+    context "when the correct user is logged in" do
+      it "displays the edit user form" do
+        get :edit, params: { id: user.username }, session: { user_id: user.id }
+        expect(response).to have_http_status(:success)
+        expect(response).to render_template(:edit)
+      end
+    end
+
+    context "when an different user is logged in" do
+      let(:other_user) { create(:user) }
+      it "redirects to the ui users page" do
+        get :edit, params: { id: user.username }, session: { user_id: other_user.id }
+        expect(response).to redirect_to(ui_user_path(user.username))
+        expect(flash[:alert]).to be_present
+      end
+    end
+
+    context "when no user is logged in" do
+      it "redirects to the login page" do
+        get :edit, params: { id: user.username }, session: { user_id: nil }
+        expect(response).to redirect_to(login_path)
+        expect(flash[:alert]).to be_present
       end
     end
+  end
 
+  describe "update" do
+    context "when the correct user is logged in" do
+      context "when the provided data is valid" do
+        it "updates the user and redirects back to the profile" do
+          put :update,
+            params: {
+              id: user.username,
+              user: { city: "Tarragona" }
+            },
+            session: { user_id: user.id }
+          expect(response).to redirect_to(ui_user_path(user.username))
+          expect(flash[:success]).to be_present
+        end
+      end
+
+      context "when the provided data is invalid" do
+        it "does not update the user, and redirects back to the form" do
+          old_email = user.email
+          put :update,
+            params: {
+              id: user.username,
+              user: { email: "not_an_email"}
+            },
+            session: { user_id: user.id }
+          expect(user.reload.email).to eq(old_email)
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(response).to render_template(:new)
+          expect(flash[:alert]).to be_present
+        end
+      end
+    end
+
+    context "when a different user is logged in" do
+      let(:other_user) { create(:user) }
+      it "does not update the user and redirects to the user profile page" do
+        expect_any_instance_of(User).not_to receive(:update)
+        put :update,
+          params: { id: user.username, user: { city: "Tarragona", country_code: "ES" } },
+          session: { user_id: other_user.id }
+        expect(response).to redirect_to(ui_user_path(user.username))
+        expect(flash[:alert]).to be_present
+      end
+    end
+
+    context "whn no user is logged in" do
+      it "does not update the user and redirects to the user profile page" do
+        expect_any_instance_of(User).not_to receive(:update)
+        put :update,
+          params: { id: user.username, user: { city: "Tarragona", country_code: "ES" } },
+          session: { user_id: nil }
+        expect(response).to redirect_to(login_path)
+        expect(flash[:alert]).to be_present
+      end
+    end
+  end
 
   describe "delete" do
     context "when the correct user is logged in" do
       it "displays the delete user form" do
-        get :delete, params: { id: user.id }, session: { user_id: user.id }
+        get :delete, params: { id: user.username }, session: { user_id: user.id }
         expect(response).to have_http_status(:success)
         expect(response).to render_template(:delete)
       end
@@ -102,28 +185,28 @@
     context "when a different user is logged in" do
       let(:other_user) { create(:user) }
       it "redirects to the ui users page" do
-        get :delete, params: { id: user.id }, session: { user_id: other_user.id }
+        get :delete, params: { id: user.username }, session: { user_id: other_user.id }
         expect(response).to redirect_to(ui_users_path)
         expect(flash[:alert]).to be_present
       end
     end
 
     context "when no user is logged in" do
       it "redirects to the login page" do
-        get :delete, params: { id: user.id }, session: { user_id: nil }
+        get :delete, params: { id: user.username }, session: { user_id: nil }
         expect(response).to redirect_to(login_path)
         expect(flash[:alert]).to be_present
       end
     end
-    end
+  end
 
   describe "destroy" do
     context "when the correct user is logged in" do
       context "when the correct username is provided" do
         it "archives the user, logs out and redirects to the post delete page" do
           expect_any_instance_of(User).to receive(:archive!)
           delete :destroy,
-            params: { id: user.id, username: user.username },
+            params: { id: user.username, username: user.username },
             session: { user_id: user.id }
           expect(response).to redirect_to(post_delete_ui_users_path)
           expect(session[:user_id]).to be_nil
@@ -134,14 +217,14 @@
         it "does not archive the user and redirects to the delete page" do
           expect_any_instance_of(User).not_to receive(:archive!)
           delete :destroy,
-            params: { id: user.id, username: "a wrong username" },
+            params: { id: user.username, username: "a wrong username" },
             session: { user_id: user.id }
           expect(response).to redirect_to(delete_ui_user_path(user.username))
           expect(flash[:alert]).to be_present
           expect(session[:user_id]).to eq(user.id)
         end
       end
-      end
+    end
 
     context "when a different user is logged in" do
 
@@ -150,7 +233,7 @@
       it "does not archive the user and redirects to the ui users page" do
         expect_any_instance_of(User).not_to receive(:archive!)
         delete :destroy,
-          params: { id: user.id, username: user.username },
+          params: { id: user.username, username: user.username },
           session: { user_id: other_user.id }
         expect(response).to redirect_to(ui_users_path)
         expect(flash[:alert]).to be_present
@@ -162,14 +245,14 @@
       it "does not archive the user and redirets to the login page" do
         expect_any_instance_of(User).not_to receive(:archive!)
         delete :destroy,
-          params: { id: user.id, username: user.username },
+          params: { id: user.username, username: user.username },
           session: { user_id: nil }
         expect(response).to redirect_to(login_path)
         expect(flash[:alert]).to be_present
         expect(session[:user_id]).to be_nil
       end
     end
-    end
+  end
 
   describe "post_delete" do
     it "renders the template" do