From 86981128b83735937627c4bb034d336a03ae4da8 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Mon, 10 Feb 2025 16:34:32 +0100 Subject: [PATCH] [#60151] added unit tests for new strategies --- .../storage_interaction/authentication.rb | 2 + .../authentication_strategies/failure.rb | 52 +++++++++++ .../nextcloud_strategies.rb | 27 +++--- .../one_drive_strategies.rb | 4 +- .../sso_user_token.rb | 3 +- .../storages/app/models/storages/storage.rb | 2 +- .../nextcloud_strategies/user_bound_spec.rb | 86 +++++++++++++++++++ .../sso_user_token_spec.rb | 79 +++++++++++++++++ spec/factories/user_factory.rb | 3 + 9 files changed, 242 insertions(+), 16 deletions(-) create mode 100644 modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/failure.rb create mode 100644 modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_strategies/nextcloud_strategies/user_bound_spec.rb create mode 100644 modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_strategies/sso_user_token_spec.rb diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication.rb index 19494f7c514..34650890de2 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication.rb @@ -38,6 +38,8 @@ module Storages case strategy.key when :noop AuthenticationStrategies::Noop.new + when :failure + AuthenticationStrategies::Failure.new when :basic_auth AuthenticationStrategies::BasicAuth.new when :sso_user_token diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/failure.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/failure.rb new file mode 100644 index 00000000000..bc0c5fa3f23 --- /dev/null +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/failure.rb @@ -0,0 +1,52 @@ +# frozen_string_literal:true + +#-- 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. +#++ + +module Storages + module Peripherals + module StorageInteraction + module AuthenticationStrategies + class Failure + def self.strategy + Strategy.new(:failure) + end + + # rubocop:disable Lint/UnusedMethodArgument + def call(storage:, http_options: {}) + data = ::Storages::StorageErrorData.new(source: self.class) + log_message = "Authentication was forced to fail. No request executed." + Failures::Builder.call(code: :error, log_message:, data:) + end + + # rubocop:enable Lint/UnusedMethodArgument + end + end + end + end +end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/nextcloud_strategies.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/nextcloud_strategies.rb index 69ea9acd007..ba325f17f73 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/nextcloud_strategies.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/nextcloud_strategies.rb @@ -33,23 +33,28 @@ module Storages module StorageInteraction module AuthenticationStrategies module NextcloudStrategies + extend TaggedLogging + UserLess = -> do ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::BasicAuth.strategy end UserBound = ->(user:, storage:) do - sso_preferred = storage.audience.present? && user.authentication_provider.present? + with_tagged_logger do + sso_preferred = storage.audience.present? && user.authentication_provider.present? - if sso_preferred - ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::SsoUserToken - .strategy - .with_user(user) - elsif storage.oauth_configuration.present? - ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken - .strategy - .with_user(user) - else - ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::Noop.strategy + if sso_preferred + ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::SsoUserToken + .strategy + .with_user(user) + elsif storage.oauth_client.present? + ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken + .strategy + .with_user(user) + else + error "No user-bound authentication strategy applicable for file storage #{storage.id}." + ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::Failure.strategy + end end end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/one_drive_strategies.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/one_drive_strategies.rb index 7833c61c6c1..cd751169908 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/one_drive_strategies.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/one_drive_strategies.rb @@ -37,13 +37,11 @@ module Storages ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthClientCredentials.strategy end - # rubocop:disable Lint/UnusedBlockArgument - UserBound = ->(user:, storage:) do + UserBound = ->(user:, storage:) do # rubocop:disable Lint/UnusedBlockArgument ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken .strategy .with_user(user) end - # rubocop:enable Lint/UnusedBlockArgument end end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/sso_user_token.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/sso_user_token.rb index 6e817e5ddc7..3e02634232c 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/sso_user_token.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/sso_user_token.rb @@ -51,7 +51,8 @@ module Storages yield OpenProject.httpx.with(opts) end, ->(error) do - Failures::Builder.call(code: :unauthorized, log_message: error.message) + log_message = "Failed to fetch access token for user #{@user}. Error: #{error.inspect}" + Failures::Builder.call(code: :unauthorized, log_message:, data: error) end ) end diff --git a/modules/storages/app/models/storages/storage.rb b/modules/storages/app/models/storages/storage.rb index ae6530096f5..7ccee78c7f2 100644 --- a/modules/storages/app/models/storages/storage.rb +++ b/modules/storages/app/models/storages/storage.rb @@ -149,7 +149,7 @@ module Storages # Returns a value of an audience, if configured for this storage. # The presence of an audience signals that this storage prioritizes - # remote authentication over token exchange if possible. + # remote authentication via Single-Sign-On if possible. def audience raise Errors::SubclassResponsibility end diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_strategies/nextcloud_strategies/user_bound_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_strategies/nextcloud_strategies/user_bound_spec.rb new file mode 100644 index 00000000000..b5424ad0997 --- /dev/null +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_strategies/nextcloud_strategies/user_bound_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +#-- 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. +#++ + +require "spec_helper" +require_module_spec_helper + +RSpec.describe Storages::Peripherals::StorageInteraction::AuthenticationStrategies::NextcloudStrategies::UserBound do + context "if file storage is configured for sso and oauth and user is provisioned by an IDP" do + let(:provider) { create(:oidc_provider) } + let(:user) { create(:user, identity_url: "#{provider.slug}:me") } + let(:storage) { create(:nextcloud_storage_configured) } + + it "must use an SsoUserToken strategy" do + strategy = described_class.call(user:, storage:) + expect(strategy.key).to eq(:sso_user_token) + end + end + + context "if file storage is configured for sso and oauth and user is local" do + let(:user) { create(:user) } + let(:storage) { create(:nextcloud_storage_configured) } + + it "must use an OAuthUserToken strategy" do + strategy = described_class.call(user:, storage:) + expect(strategy.key).to eq(:oauth_user_token) + end + end + + context "if file storage is configured only for oauth and user is provided by an IDP" do + let(:provider) { create(:oidc_provider) } + let(:user) { create(:user, identity_url: "#{provider.slug}:me") } + let(:storage) { create(:nextcloud_storage_configured, nextcloud_audience: nil) } + + it "must use an OAuthUserToken strategy" do + strategy = described_class.call(user:, storage:) + expect(strategy.key).to eq(:oauth_user_token) + end + end + + context "if file storage is configured only for oauth and user is local" do + let(:user) { create(:user) } + let(:storage) { create(:nextcloud_storage_configured, nextcloud_audience: nil) } + + it "must use an OAuthUserToken strategy" do + strategy = described_class.call(user:, storage:) + expect(strategy.key).to eq(:oauth_user_token) + end + end + + context "if file storage is not fully configured" do + let(:user) { create(:user) } + let(:storage) { create(:nextcloud_storage, nextcloud_audience: nil) } + + it "must return the failure strategy" do + strategy = described_class.call(user:, storage:) + expect(strategy.key).to eq(:failure) + end + end +end diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_strategies/sso_user_token_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_strategies/sso_user_token_spec.rb new file mode 100644 index 00000000000..e00c48648ed --- /dev/null +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_strategies/sso_user_token_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +#-- 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. +#++ + +require "spec_helper" +require_module_spec_helper + +RSpec.describe Storages::Peripherals::StorageInteraction::AuthenticationStrategies::SsoUserToken do + include Dry::Monads[:result] + + let(:storage) { create(:nextcloud_storage) } + + subject(:strategy) { described_class.new(create(:user)) } + + before do + service = instance_double(OpenIDConnect::UserTokens::FetchService) + allow(OpenIDConnect::UserTokens::FetchService).to receive(:new).and_return(service) + allow(service).to receive(:access_token_for).with(audience: storage.audience).and_return(access_token_result) + end + + context "if access token can be fetched successfully" do + let(:token) { "my_access_token" } + let(:access_token_result) { Success(token) } + + it "must yield with access token" do + was_yielded = false + + strategy.call(storage:) do |http| + was_yielded = true + expect(http.instance_variable_get(:@options).headers["authorization"]).to eq("Bearer #{token}") + end + + expect(was_yielded).to be_truthy + end + end + + context "if fetching access token fails" do + let(:error) { "U shall not pass!" } + let(:access_token_result) { Failure(error) } + + it "must not yield and return failure" do + was_yielded = false + result = strategy.call(storage:) { was_yielded = true } + + expect(was_yielded).to be_falsy + expect(result).to be_failure + expect(result.result).to eq(:unauthorized) + expect(result.errors).to be_a(Storages::StorageError) + expect(result.errors.data).to eq(error) + expect(result.errors.data).to eq(error) + end + end +end diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 9bf5bc3dc40..53a62955634 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -34,6 +36,7 @@ FactoryBot.define do sequence(:mail) { |n| "bobmail#{n}.bobbit@bob.com" } password { "adminADMIN!" } password_confirmation { "adminADMIN!" } + identity_url { nil } transient do preferences { {} }