From 58c576f55df156aa0e9b8b8523ca3063946d1bff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 25 Oct 2024 17:37:07 +0200 Subject: [PATCH 1/2] Remove unused sys actions These actions were never documented except for fetch_changesets, and we can use the 15.0 release to remove support for it. --- app/controllers/sys_controller.rb | 88 +----- .../repositories_settings/show.html.erb | 1 - config/constants/settings/definition.rb | 3 - config/locales/en.yml | 1 - config/routes.rb | 6 +- .../configuration/repositories/README.md | 4 - extra/svn/reposman.rb | 52 ---- spec/controllers/sys_controller_spec.rb | 266 ------------------ 8 files changed, 4 insertions(+), 417 deletions(-) delete mode 100755 extra/svn/reposman.rb diff --git a/app/controllers/sys_controller.rb b/app/controllers/sys_controller.rb index c2d0bd085e7d..b12087027fee 100644 --- a/app/controllers/sys_controller.rb +++ b/app/controllers/sys_controller.rb @@ -33,46 +33,6 @@ class SysController < ActionController::Base before_action :check_enabled before_action :require_basic_auth, only: [:repo_auth] - before_action :find_project, only: [:update_required_storage] - before_action :find_repository_with_storage, only: [:update_required_storage] - - def projects - p = Project.active.has_module(:repository) - .includes(:repository) - .references(:repositories) - .order(Arel.sql("identifier")) - respond_to do |format| - format.json do - render json: p.to_json(include: :repository) - end - format.any(:html, :xml) do - render xml: p.to_xml(include: :repository), content_type: Mime[:xml] - end - end - end - - def update_required_storage - result = update_storage_information(@repository, params[:force] == "1") - render plain: "Updated: #{result}", status: :ok - end - - def fetch_changesets - projects = [] - if params[:id] - projects << Project.active.has_module(:repository).find_by!(identifier: params[:id]) - else - projects = Project.active.has_module(:repository) - .includes(:repository).references(:repositories) - end - projects.each do |project| - if project.repository - project.repository.fetch_changesets - end - end - head :ok - rescue ActiveRecord::RecordNotFound - head :not_found - end def repo_auth project = Project.find_by(identifier: params[:repository]) @@ -104,38 +64,9 @@ def check_enabled end end - def update_storage_information(repository, force = false) - if force - ::SCM::StorageUpdaterJob.perform_later(repository) - true - else - repository.update_required_storage - end - end - - def find_project - @project = Project.find(params[:id]) - rescue ActiveRecord::RecordNotFound - render plain: "Could not find project ##{params[:id]}.", status: :not_found - end - - def find_repository_with_storage - @repository = @project.repository - - if @repository.nil? - render plain: "Project ##{@project.id} does not have a repository.", status: :not_found - else - return true if @repository.scm.storage_available? - - render plain: "repositories.storage.not_available", status: :bad_request - end - - false - end - def require_basic_auth authenticate_with_http_basic do |username, password| - @authenticated_user = cached_user_login(username, password) + @authenticated_user = user_login(username, password) return true if @authenticated_user end @@ -147,21 +78,4 @@ def require_basic_auth def user_login(username, password) User.try_to_login(username, password) end - - def cached_user_login(username, password) - unless Setting.repository_authentication_caching_enabled? - return user_login(username, password) - end - - user = nil - user_id = Rails.cache.fetch(OpenProject::RepositoryAuthentication::CACHE_PREFIX + Digest::SHA1.hexdigest("#{username}#{password}"), - expires_in: OpenProject::RepositoryAuthentication::CACHE_EXPIRES_AFTER) do - user = user_login(username, password) - user ? user.id.to_s : "-1" - end - - return nil if user_id.blank? or user_id == "-1" - - user || User.find_by(id: user_id.to_i) - end end diff --git a/app/views/admin/settings/repositories_settings/show.html.erb b/app/views/admin/settings/repositories_settings/show.html.erb index 316be6b0bdd6..b6880007ae69 100644 --- a/app/views/admin/settings/repositories_settings/show.html.erb +++ b/app/views/admin/settings/repositories_settings/show.html.erb @@ -101,7 +101,6 @@ See COPYRIGHT and LICENSE files for more details.
<%= setting_text_field :repository_log_display_limit, size: 6, container_class: '-xslim' %>
<%= setting_text_field :repository_truncate_at, size: 6, container_class: '-xslim' %>
-
<%= setting_check_box :repository_authentication_caching_enabled %>
<%= render partial: 'repositories_checkout' %>
diff --git a/config/constants/settings/definition.rb b/config/constants/settings/definition.rb index 715707dd6b73..16fe9e0705f2 100644 --- a/config/constants/settings/definition.rb +++ b/config/constants/settings/definition.rb @@ -877,9 +877,6 @@ class Definition default: nil, format: :string }, - repository_authentication_caching_enabled: { - default: true - }, repository_checkout_data: { default: { "git" => { "enabled" => 0 }, diff --git a/config/locales/en.yml b/config/locales/en.yml index e9929d2ab022..3aee26cb3e2d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3412,7 +3412,6 @@ en: setting_registration_footer: "Registration footer" setting_repositories_automatic_managed_vendor: "Automatic repository vendor type" setting_repositories_encodings: "Repositories encodings" - setting_repository_authentication_caching_enabled: "Enable caching for authentication request of version control software" setting_repository_storage_cache_minutes: "Repository disk size cache" setting_repository_checkout_display: "Show checkout instructions" setting_repository_checkout_base_url: "Checkout base URL" diff --git a/config/routes.rb b/config/routes.rb index eff38e6da7ad..ff70c59a6c2f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -694,9 +694,9 @@ scope controller: "sys" do match "/sys/repo_auth", action: "repo_auth", via: %i[get post] - get "/sys/projects", action: "projects" - get "/sys/fetch_changesets", action: "fetch_changesets" - get "/sys/projects/:id/repository/update_storage", action: "update_required_storage" + match "/sys/projects", to: proc { [410, {}, [""]] }, via: :all + match "/sys/fetch_changesets", to: proc { [410, {}, [""]] }, via: :all + match "/sys/projects/:id/repository/update_storage", to: proc { [410, {}, [""]] }, via: :all end # alternate routes for the current user diff --git a/docs/installation-and-operations/configuration/repositories/README.md b/docs/installation-and-operations/configuration/repositories/README.md index 580901ac6548..f335a2789d8b 100644 --- a/docs/installation-and-operations/configuration/repositories/README.md +++ b/docs/installation-and-operations/configuration/repositories/README.md @@ -162,10 +162,6 @@ The total required disk space for a project's its repository and attachments are This information is refreshed in the same manner that changesets are retrieved: By default, the repository is refreshed when a user visits the repository page. This information is cached for the time configured under the global `administration settings → repositories`. -It could also externally be refreshed by using a cron job using the Sys API. Executing a GET against `/sys/projects/:identifier/repository/update_storage` will cause a refresh when the maximum cache time is expired. If you pass the query `?force=1` to the request above, it will ignore the cache. - -For a future release, we are hoping to provide a webhook to update changesets and storage immediately after a change has been committed to the repository. - ## Accessing repositories through Apache With managed repositories, OpenProject takes care of the lifetime of repositories and their association with projects, however we still need to serve the repositories to the client. diff --git a/extra/svn/reposman.rb b/extra/svn/reposman.rb deleted file mode 100755 index d154767c6e1d..000000000000 --- a/extra/svn/reposman.rb +++ /dev/null @@ -1,52 +0,0 @@ -#!/usr/bin/env ruby -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) the OpenProject GmbH -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License version 3. -# -# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -# Copyright (C) 2006-2013 Jean-Philippe Lang -# Copyright (C) 2010-2013 the ChiliProject Team -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -# -# See COPYRIGHT and LICENSE files for more details. -#++ - -warn <<~EOS - [DEPRECATION] The functionality provided by reposman.rb has been integrated into OpenProject. - Please remove any existing cronjobs that still use this script. - #{' '} - You can create repositories explicitly on the filesystem using managed repositories. - Enable managed repositories for each SCM vendor individually using the templates - defined in configuration.yml. - #{' '} - If you want to convert existing repositories previously created (by reposman.rb or manually) - into managed repositories, use the following command: - #{' '} - $ bundle exec rake scm:migrate:managed[URL prefix (, URL prefix, ...)] - Where URL prefix denotes a common prefix of repositories whose status should be upgraded to :managed. - Example: - #{' '} - If you have executed reposman.rb with the following parameters: - #{' '} - $ reposman.rb [...] --svn-dir "/opt/svn" --url "file:///opt/svn" - #{' '} - Then you can pass a URL prefix of 'file:///opt/svn' and the rake task will migrate all repositories - matching this prefix to :managed. - You may pass more than one URL prefix to the task. -EOS diff --git a/spec/controllers/sys_controller_spec.rb b/spec/controllers/sys_controller_spec.rb index fafd1543312a..00b3080e3c66 100644 --- a/spec/controllers/sys_controller_spec.rb +++ b/spec/controllers/sys_controller_spec.rb @@ -55,7 +55,6 @@ DeletedUser.first # creating it first in order to avoid problems with should_receive allow(Setting).to receive(:sys_api_key).and_return(api_key) - allow(Setting).to receive(:repository_authentication_caching_enabled?).and_return(true) Rails.cache.clear RequestStore.clear! @@ -494,269 +493,4 @@ end end end - - describe "#cached_user_login" do - let(:cache_key) do - OpenProject::RepositoryAuthentication::CACHE_PREFIX + - Digest::SHA1.hexdigest("#{valid_user.login}#{valid_user_password}") - end - let(:cache_expiry) { OpenProject::RepositoryAuthentication::CACHE_EXPIRES_AFTER } - - it "calls user_login only once when called twice" do - expect(controller).to receive(:user_login).once.and_return(valid_user) - 2.times { controller.send(:cached_user_login, valid_user.login, valid_user_password) } - end - - it "returns the same as user_login for valid creds" do - expect(controller.send(:cached_user_login, valid_user.login, valid_user_password)) - .to eq(controller.send(:user_login, valid_user.login, valid_user_password)) - end - - it "returns the same as user_login for invalid creds" do - expect(controller.send(:cached_user_login, "invalid", "invalid")) - .to eq(controller.send(:user_login, "invalid", "invalid")) - end - - it "uses cache" do - allow(Rails.cache).to receive(:fetch).and_call_original - expect(Rails.cache).to receive(:fetch).with(cache_key, expires_in: cache_expiry) - .and_return(Marshal.dump(valid_user.id.to_s)) - controller.send(:cached_user_login, valid_user.login, valid_user_password) - end - - describe "with caching disabled" do - before do - allow(Setting).to receive(:repository_authentication_caching_enabled?).and_return(false) - end - - it "does not use a cache" do - allow(Rails.cache).to receive(:fetch).and_wrap_original do |m, *args, &block| - expect(args.first).not_to eq(cache_key) - m.call(*args, &block) - end - - controller.send(:cached_user_login, valid_user.login, valid_user_password) - end - end - - describe "update_required_storage" do - let(:force) { nil } - let(:apikey) { Setting.sys_api_key } - let(:last_updated) { nil } - - def request_storage - get "update_required_storage", params: { key: apikey, - id:, - force: } - end - - context "missing project" do - let(:id) { 1234 } - - it "returns 404" do - request_storage - expect(response.code).to eq("404") - expect(response.body).to include("Could not find project #1234") - end - end - - context "available project, but missing repository" do - let(:project) { build_stubbed(:project) } - let(:id) { project.id } - - before do - allow(Project).to receive(:find).and_return(project) - request_storage - end - - it "returns 404" do - expect(response.code).to eq("404") - expect(response.body).to include("Project ##{project.id} does not have a repository.") - end - end - - context "stubbed repository" do - let(:project) { build_stubbed(:project) } - let(:id) { project.id } - let(:repository) do - build_stubbed(:repository_subversion, url:, root_url: url) - end - - before do - allow(Project).to receive(:find).and_return(project) - allow(project).to receive(:repository).and_return(repository) - - allow(repository).to receive(:storage_updated_at).and_return(last_updated) - request_storage - end - - context "local non-existing repository" do - let(:root_url) { "/tmp/does/not/exist/svn/foo.svn" } - let(:url) { "file://#{root_url}" } - - it "does not have storage available" do - expect(repository.scm.storage_available?).to be false - expect(response.code).to eq("400") - end - end - - context "remote stubbed repository" do - let(:root_url) { "" } - let(:url) { "https://foo.example.org/svn/bar" } - - it "has no storage available" do - request_storage - expect(repository.scm.storage_available?).to be false - expect(response.code).to eq("400") - end - end - end - - context "local existing repository" do - with_subversion_repository do |repo_dir| - let(:root_url) { repo_dir } - let(:url) { "file://#{root_url}" } - - let(:project) { create(:project) } - let(:id) { project.id } - let(:repository) do - create(:repository_subversion, project:, url:, root_url: url) - end - - before do - allow(Project).to receive(:find).and_return(project) - allow(project).to receive(:repository).and_return(repository) - allow(repository).to receive(:storage_updated_at).and_return(last_updated) - end - - it "has storage available" do - expect(repository.scm.storage_available?).to be true - end - - context "storage never updated before" do - it "updates the storage" do - expect(repository.required_storage_bytes).to eq 0 - request_storage - - expect(response.code).to eq("200") - expect(response.body).to include("Updated: true") - - perform_enqueued_jobs - - repository.reload - expect(repository.required_storage_bytes).to be > 0 - end - end - - context "outdated storage" do - let(:last_updated) { 2.days.ago } - - it "updates the storage" do - expect(SCM::StorageUpdaterJob).to receive(:perform_later) - request_storage - end - end - - context "valid storage time" do - let(:last_updated) { 10.minutes.ago } - - it "does not update to storage" do - expect(SCM::StorageUpdaterJob).not_to receive(:perform_later) - request_storage - end - end - - context "valid storage time and force" do - let(:force) { "1" } - let(:last_updated) { 10.minutes.ago } - - it "does update to storage" do - expect(SCM::StorageUpdaterJob).to receive(:perform_later) - request_storage - end - end - end - end - end - end - - describe "#projects" do - before do - request.env["HTTP_AUTHORIZATION"] = - ActionController::HttpAuthentication::Basic.encode_credentials( - valid_user.login, - valid_user_password - ) - - get "projects", params: { key: api_key } - end - - it "is successful" do - expect(response) - .to have_http_status(:ok) - end - - it "returns an xml with the projects having a repository" do - expect(response.content_type) - .to eql "application/xml; charset=utf-8" - - expect(Nokogiri::XML::Document.parse(response.body).xpath("//projects[1]//id").text) - .to eql repository_project.id.to_s - end - - context "when disabled", with_settings: { sys_api_enabled?: false } do - it "is 403 forbidden" do - expect(response) - .to have_http_status(:forbidden) - end - end - end - - describe "#fetch_changesets" do - let(:params) { { id: repository_project.identifier } } - - before do - request.env["HTTP_AUTHORIZATION"] = - ActionController::HttpAuthentication::Basic.encode_credentials( - valid_user.login, - valid_user_password - ) - - allow_any_instance_of(Repository::Subversion).to receive(:fetch_changesets).and_return(true) - - get "fetch_changesets", params: params.merge({ key: api_key }) - end - - context "with a project identifier" do - it "is successful" do - expect(response) - .to have_http_status(:ok) - end - end - - context "without a project identifier" do - let(:params) { {} } - - it "is successful" do - expect(response) - .to have_http_status(:ok) - end - end - - context "for an unknown project" do - let(:params) { { id: 0 } } - - it "returns 404" do - expect(response) - .to have_http_status(:not_found) - end - end - - context "when disabled", with_settings: { sys_api_enabled?: false } do - it "is 403 forbidden" do - expect(response) - .to have_http_status(:forbidden) - end - end - end end From 6f4c7fa99d5002cc60a99af692106ba395330a33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 28 Oct 2024 08:16:37 +0100 Subject: [PATCH 2/2] Remove useless skip_action for tests --- modules/backlogs/app/controllers/rb_application_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/backlogs/app/controllers/rb_application_controller.rb b/modules/backlogs/app/controllers/rb_application_controller.rb index fa69a08119b8..37e0502db725 100644 --- a/modules/backlogs/app/controllers/rb_application_controller.rb +++ b/modules/backlogs/app/controllers/rb_application_controller.rb @@ -32,8 +32,6 @@ class RbApplicationController < ApplicationController before_action :load_sprint_and_project, :check_if_plugin_is_configured, :authorize - skip_before_action :verify_authenticity_token, if: -> { Rails.env.test? } - # Use special backlogs layout to initialize stimulus side-loading legacy backlogs scripts # and CSS from frontend layout "backlogs"