Backport 'Don't allow access to admin panel without ToS acceptance' t…

…o v0.26 (#11047)

* Don't allow access to admin panel without ToS acceptance

* Add redirection to previous page after accepting ToS

* Use have_content instead of have_text

* Running spellcheck linters

* Fix specs

* Fix permissions on Templates when user is not admin

* Fix specs

* Fix i18n string scope from merge

* Workaround for admin ToS acceptance in Initiatives

After #5736, the initiatives' authors and commitee members should not
have access to the admin panel.

The problem is that with the change of the Terms of Service acceptance
in the admin panel this is changing, so there's still some leftovers in
the initiatives' permissions.

As I only want to focus on ToS acceptance for now, I'll skip these specs
and fix the real problem (cleaning the leftovers from #5736) on another
PR to keep this small.

* Fix typo

* Fix for possible Cookie overflow with a long list of URL params

Detected by code review

* Remove unecessary namespaces

* Fix spec

* Bring consistency to the spec messages

* "has not accepted" sounds better than "did not accepted"
* sometimes I was using "has a message" and other times "shows a
  message"
* sometimes we were using ToS and other times TOS

* Add missing specs for Templates' specs

* Remove unecessary return

Apply suggestions from code review



* Fix the stored request.path to not mess with the frontend's stored location

Apply suggestions from code review



* Fetch from stored_location_for so the session value is cleaned

Apply suggestions from code review



* Fix traits usages in factories calls

Apply suggestions from code review



* Introduce "needs admin TOS accepted" shared example

* Fix rubocop offenses

* Fix rubocop offenses

* Make the user configurable for "needs admin TOS accepted" shared example

* Fix rubocop offense

* Refactor spec to shared examples

* Add example for roles that aren't admin

---------

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

decidim-admin/app/controllers/concerns/decidim/admin/needs_admin_tos_accepted.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module Decidim
+  module Admin
+    # Shared behaviour for signed_in admins that require the latest TOS accepted
+    module NeedsAdminTosAccepted
+      extend ActiveSupport::Concern
+
+      included do
+        before_action :tos_accepted_by_admin
+      end
+
+      private
+
+      def tos_accepted_by_admin
+        return unless request.format.html?
+        return unless current_user
+        return if current_user.admin_terms_accepted?
+        return if permitted_paths?
+
+        store_location_for(
+          current_user,
+          request.path
+        )
+        redirect_to admin_tos_path
+      end
+
+      def permitted_paths?
+        # ensure that path with or without query string pass
+        permitted_paths.find { |el| el.split("?").first == request.path }
+      end
+
+      def permitted_paths
+        [admin_tos_path, decidim_admin.admin_terms_accept_path]
+      end
+
+      def admin_tos_path
+        decidim_admin.admin_terms_show_path
+      end
+    end
+  end
+end

decidim-admin/app/controllers/decidim/admin/admin_terms_controller.rb

@@ -9,7 +9,7 @@ def accept
         current_user.admin_terms_accepted_at = Time.current
         if current_user.save!
           flash[:notice] = t("accept.success", scope: "decidim.admin.admin_terms_of_use")
-          redirect_to decidim_admin.root_path
+          redirect_to stored_location_for(current_user) || decidim_admin.root_path
         else
           flash[:alert] = t("accept.error", scope: "decidim.admin.admin_terms_of_use")
           redirect_to decidim_admin.admin_terms_show_path

decidim-admin/app/controllers/decidim/admin/application_controller.rb

@@ -7,6 +7,7 @@ class ApplicationController < ::DecidimController
       include NeedsOrganization
       include NeedsPermission
       include NeedsSnippets
+      include NeedsAdminTosAccepted
       include FormFactory
       include LocaleSwitcher
       include UseOrganizationTimeZone

decidim-admin/lib/decidim/admin/test.rb

@@ -9,3 +9,4 @@
 require "decidim/admin/test/filterable_examples"
 require "decidim/admin/test/filters_participatory_space_users_examples"
 require "decidim/admin/test/filters_participatory_space_user_roles_examples"
+require "decidim/admin/test/needs_admin_tos_accepted_examples"

decidim-admin/lib/decidim/admin/test/needs_admin_tos_accepted_examples.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+shared_examples_for "needs admin TOS accepted" do
+  context "when the user has not accepted the admin TOS" do
+    it "shows a message to accept the admin TOS" do
+      expect(page).to have_content("Please take a moment to review Admin Terms of Use")
+    end
+  end
+end

decidim-admin/spec/controllers/concerns/needs_admin_tos_accepted_spec.rb

@@ -0,0 +1,120 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+module Decidim
+  module Admin
+    describe "NeedsAdminTosAccepted", type: :controller do
+      let!(:organization) { create(:organization) }
+
+      controller do
+        include NeedsAdminTosAccepted
+
+        def root
+          render plain: "Root page"
+        end
+
+        def admin_tos
+          render plain: "Admin TOS page"
+        end
+
+        def another
+          render plain: "Another page"
+        end
+
+        private
+
+        def permitted_paths
+          ["/root", "/admin_tos"]
+        end
+
+        def admin_tos_path
+          "/admin_tos"
+        end
+      end
+
+      before do
+        routes.draw do
+          get "root" => "anonymous#root"
+          get "another" => "anonymous#another"
+          get "admin_tos" => "anonymous#admin_tos"
+        end
+
+        request.env["decidim.current_organization"] = organization
+        sign_in user, scope: :user
+      end
+
+      shared_examples "needs admins' TOS acceptance to access other pages" do
+        it "allows accessing the root page" do
+          get :root
+
+          expect(response.body).to have_text("Root page")
+        end
+
+        it "allows accessing the TOS page" do
+          get :admin_tos
+
+          expect(response.body).to have_text("Admin TOS page")
+        end
+
+        it "does not allow accessing another page" do
+          get :another
+
+          expect(response).to redirect_to("/admin_tos")
+          expect(response.body).to have_text("You are being redirected")
+          expect(session[:user_return_to]).to eq("/another")
+        end
+      end
+
+      shared_examples "allows accessing all the pages" do
+        it "allows accessing the root page" do
+          get :root
+
+          expect(response.body).to have_text("Root page")
+        end
+
+        it "allows accessing the TOS page" do
+          get :admin_tos
+
+          expect(response.body).to have_text("Admin TOS page")
+        end
+
+        it "allows accessing another page" do
+          get :another
+
+          expect(response.body).to have_text("Another page")
+        end
+      end
+
+      context "when the user is an admin" do
+        context "and has not accepted the TOS" do
+          let(:user) { create(:user, :admin, :confirmed, admin_terms_accepted_at: nil, organization: organization) }
+
+          it_behaves_like "needs admins' TOS acceptance to access other pages"
+        end
+
+        context "and has accepted the TOS" do
+          let(:user) { create(:user, :admin, :confirmed) }
+
+          it_behaves_like "allows accessing all the pages"
+        end
+      end
+
+      context "when the user has another role with access to admin panel" do
+        let(:participatory_process) { create(:participatory_process, organization: organization) }
+
+        context "and has not accepted the TOS" do
+          let(:user) { create(:process_moderator, confirmed_at: Time.current, admin_terms_accepted_at: nil, participatory_process: participatory_process) }
+
+          it_behaves_like "needs admins' TOS acceptance to access other pages"
+        end
+
+        context "and has accepted the TOS" do
+          let(:user) { create(:process_moderator, confirmed_at: Time.current, participatory_process: participatory_process) }
+
+          it_behaves_like "allows accessing all the pages"
+        end
+      end
+    end
+  end
+end

decidim-admin/spec/system/admin_invite_spec.rb

@@ -55,7 +55,7 @@
       end
 
       expect(page).to have_content("Dashboard")
-      expect(page).to have_current_path "/admin/"
+      expect(page).to have_current_path "/admin/admin_terms/show"
     end
   end
 end

decidim-admin/spec/system/admin_tos_acceptance_spec.rb

@@ -5,6 +5,7 @@
 describe "AdminTosAcceptance", type: :system do
   let(:organization) { create(:organization) }
   let(:user) { create(:user, :admin, :confirmed, admin_terms_accepted_at: nil, organization: organization) }
+  let(:review_message) { "Please take a moment to review Admin Terms of Use. Otherwise you won't be able to manage the platform" }
 
   before do
     switch_to_host(organization.host)
@@ -15,18 +16,18 @@
       login_as user, scope: :user
     end
 
-    context "when they visit the dashbaord" do
+    context "when they visit the dashboard" do
       before do
         visit decidim_admin.root_path
       end
 
       it "has a message that they need to accept the admin TOS" do
-        expect(page).to have_content("Please take a moment to review Admin Terms of Use. Otherwise you won't be able to manage the platform")
+        expect(page).to have_content(review_message)
       end
 
       it "has only the Dashboard menu item in the main navigation" do
         within ".main-nav" do
-          expect(page).to have_text("Dashboard")
+          expect(page).to have_content("Dashboard")
           expect(page).to have_selector("li a", count: 1)
         end
       end
@@ -37,9 +38,49 @@
         visit decidim_admin.newsletters_path
       end
 
-      it "says that you're not authorized" do
-        within ".callout.alert" do
-          expect(page).to have_text("You are not authorized to perform this action")
+      it "has a message that they need to accept the admin TOS" do
+        expect(page).to have_content(review_message)
+      end
+    end
+
+    context "when they visit other admin pages from other engines" do
+      before do
+        visit decidim_admin_participatory_processes.participatory_processes_path
+      end
+
+      it "has a message that they need to accept the admin TOS" do
+        expect(page).to have_content(review_message)
+      end
+
+      it "allows accepting and redirects to the previous page" do
+        click_button "I agree with the terms"
+        expect(page).to have_content("New process")
+        expect(page).to have_content("Process groups")
+      end
+
+      context "with a long list of URL parameters" do
+        let(:long_parameters) do
+          # This should generate a string of at least 4 KB in length which is
+          # the cookie session store's maximum cookie size due to browser
+          # limitations. Each parameter here is in the form of "paramxx=aaa",
+          # where "paramxx" is the parameter name and "aaa" is the value. The
+          # total length of each parameter is therefore 6 + 2 + 100 characters
+          # = 108 bytes. Cookie overflow should therefore happen at latest
+          # around 38 of these parameters concenated together.
+          50.times.map do |i|
+            "param#{i.to_s.rjust(2, "0")}=#{SecureRandom.alphanumeric(100)}"
+          end.join("&")
+        end
+
+        it "responds to requests containing very long URL parameters" do
+          # Calling any URL in Decidim with long parameters should not store
+          # the parameters in the user_return_to cookie in order to avoid
+          # ActionDispatch::Cookies::CookieOverflow exception
+          visit "#{decidim_admin_participatory_processes.participatory_processes_path}?#{long_parameters}"
+          expect(page).to have_content(review_message)
+          click_button "I agree with the terms"
+          expect(page).to have_content("New process")
+          expect(page).to have_content("Process groups")
         end
       end
     end
@@ -55,15 +96,15 @@
 
       it "allows accepting the terms" do
         click_button "I agree with the terms"
-        expect(page).to have_text("Activity")
-        expect(page).to have_text("Metrics")
+        expect(page).to have_content("Activity")
+        expect(page).to have_content("Metrics")
 
         within ".main-nav" do
-          expect(page).to have_text("Dashboard")
-          expect(page).to have_text("Newsletters")
-          expect(page).to have_text("Participants")
-          expect(page).to have_text("Settings")
-          expect(page).to have_text("Admin activity log")
+          expect(page).to have_content("Dashboard")
+          expect(page).to have_content("Newsletters")
+          expect(page).to have_content("Participants")
+          expect(page).to have_content("Settings")
+          expect(page).to have_content("Admin activity log")
         end
       end
     end

decidim-admin/spec/system/space_admin_manages_global_moderations_spec.rb

@@ -24,14 +24,13 @@
     login_as user, scope: :user
   end
 
-  context "when the user didn't accepted the admin ToS" do
+  context "when the user has not accepted the admin TOS" do
     before do
       user.update(admin_terms_accepted_at: nil)
       visit decidim_admin.moderations_path
     end
 
     it "has a message that they need to accept the admin TOS" do
-      expect(page).to have_content("You are not authorized")
       expect(page).to have_content("Please take a moment to review Admin Terms of Use. Otherwise you won't be able to manage the platform")
     end
 
@@ -47,10 +46,8 @@
         visit decidim_admin.newsletters_path
       end
 
-      it "says that you're not authorized" do
-        within ".callout.alert" do
-          expect(page).to have_text("You are not authorized to perform this action")
-        end
+      it "says that you are not authorized" do
+        expect(page).to have_text("Please take a moment to review Admin Terms of Use")
       end
     end
   end

decidim-admin/spec/system/space_moderator_manages_global_moderations_spec.rb

@@ -41,9 +41,7 @@
     it "can't access to the Global moderations page" do
       visit decidim_admin.moderations_path
 
-      within ".callout.alert" do
-        expect(page).to have_text("You are not authorized to perform this action")
-      end
+      expect(page).to have_content("Please take a moment to review Admin Terms of Use")
     end
   end
 

decidim-assemblies/spec/system/admin/valuator_checks_components_spec.rb

@@ -10,7 +10,8 @@
     decidim_admin_assemblies.components_path(assembly)
   end
   let(:components_path) { participatory_space_path }
-  let!(:user) { create :user, :confirmed, organization: organization }
+
+  let!(:user) { create :user, :confirmed, :admin_terms_accepted, admin: false, organization: organization }
   let!(:valuator_role) { create :assembly_user_role, role: :valuator, user: user, assembly: assembly }
   let(:another_component) { create :component, participatory_space: assembly }
 
@@ -19,15 +20,17 @@
   include_context "when administrating an assembly"
 
   before do
-    user.update(admin: false)
-
     create :valuation_assignment, proposal: assigned_proposal, valuator_role: valuator_role
 
     switch_to_host(organization.host)
     login_as user, scope: :user
     visit components_path
   end
 
+  it_behaves_like "needs admin TOS accepted" do
+    let(:user) { create(:user, :confirmed, organization: organization) }
+  end
+
   context "when listing the space components in the sidebar" do
     it "can only see the proposals component" do
       within ".layout-nav #components-list" do