From bd4b09592b5f9e2607c772ecdd95ddc85aafbc09 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Mon, 21 Jul 2025 08:03:18 +0200 Subject: [PATCH] Fix registration of auth providers through register_auth_providers This is the only and official API to register an auth provider. However, so far it was optional to create a database entry in the auth_providers table and only OIDC and SAML did that. On the other hand, we added expectations about auth providers have a database entry in more and more places of the codebase. Now making sure that every auth provider is represented in the database. --- app/models/auth_provider.rb | 4 -- app/models/plugin_auth_provider.rb | 48 +++++++++++++++++++ .../create-omniauth-plugin/README.md | 13 +++++ .../lib/open_project/plugins/auth_plugin.rb | 20 +++++++- .../spec/requests/auth_plugins_spec.rb | 9 +++- .../lib/open_project/auth_saml/engine.rb | 2 +- .../lib/open_project/openid_connect/engine.rb | 2 +- 7 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 app/models/plugin_auth_provider.rb diff --git a/app/models/auth_provider.rb b/app/models/auth_provider.rb index 087cd8fe95c..0ef960a072e 100644 --- a/app/models/auth_provider.rb +++ b/app/models/auth_provider.rb @@ -44,10 +44,6 @@ class AuthProvider < ApplicationRecord after_destroy :unset_direct_provider - def self.slug_fragment - raise NotImplementedError - end - def user_count @user_count ||= user_auth_provider_links.count end diff --git a/app/models/plugin_auth_provider.rb b/app/models/plugin_auth_provider.rb new file mode 100644 index 00000000000..603a45d3d0a --- /dev/null +++ b/app/models/plugin_auth_provider.rb @@ -0,0 +1,48 @@ +# 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. +#++ + +class PluginAuthProvider < AuthProvider + class << self + def create_for_plugin(config) + slug = config[:name] + display_name = config[:display_name] || slug + + find_or_create_by!(slug:) do |provider| + provider.available = false + provider.display_name = display_name.to_s + provider.creator = User.system + end + end + end + + def human_type + "Plug-in-based authentication provider" + end +end diff --git a/docs/development/create-omniauth-plugin/README.md b/docs/development/create-omniauth-plugin/README.md index 76178889753..10914c68fbb 100644 --- a/docs/development/create-omniauth-plugin/README.md +++ b/docs/development/create-omniauth-plugin/README.md @@ -103,6 +103,19 @@ module OmniAuth You can register any number of providers using different strategies (or the same) with different options. For instance you could configure two OpenID Connect providers using the same strategy (OpenIDConnect) but with different options according to the service to be used (e.g. Google vs Microsoft). +OpenProject expects a database entry in the `auth_providers` table to be stored for each provider registered for SSO login (via subclasses of the +`AuthProvider` model), so that they can be referenced, for example by users logging in through those providers. +By default the call to `register_auth_providers` will make sure that a record exists for each registered provider. +However, if your plugin manages configuration of each provider in such a subclass itself, you can pass `persist: false`, to indicate that: + +```ruby +register_auth_providers(persist: false) do + strategy :my_advanced_auth_plugin_strategy do + # ... + end +end +``` + ### Add your plugin to Gemfile.plugins All that’s that left to do is declaring your plugin in the file `Gemfile.plugins` in your OpenProject application’s root directory. diff --git a/modules/auth_plugins/lib/open_project/plugins/auth_plugin.rb b/modules/auth_plugins/lib/open_project/plugins/auth_plugin.rb index 47ae92ed70f..a852fd642c9 100644 --- a/modules/auth_plugins/lib/open_project/plugins/auth_plugin.rb +++ b/modules/auth_plugins/lib/open_project/plugins/auth_plugin.rb @@ -28,9 +28,9 @@ module OpenProject::Plugins module AuthPlugin - def register_auth_providers(&) + def register_auth_providers(persist: true, &) + builder = ProviderBuilder.new initializer "#{engine_name}.middleware" do |app| - builder = ProviderBuilder.new builder.instance_eval(&) app.config.middleware.use OmniAuth::FlexibleBuilder do @@ -39,6 +39,16 @@ module OpenProject::Plugins end end end + + config.to_prepare do + if persist + builder.provider_callbacks.each do |callback| + callback.call.each do |config| + PluginAuthProvider.create_for_plugin(config) + end + end + end + end end def self.strategies @@ -125,10 +135,16 @@ module OpenProject::Plugins AuthPlugin.strategies[key] = [providers] new_strategies << strategy end + + provider_callbacks.push(providers) end def new_strategies @new_strategies ||= [] end + + def provider_callbacks + @provider_callbacks ||= [] + end end end diff --git a/modules/auth_plugins/spec/requests/auth_plugins_spec.rb b/modules/auth_plugins/spec/requests/auth_plugins_spec.rb index 0f2ce0c4691..dc9819230cc 100644 --- a/modules/auth_plugins/spec/requests/auth_plugins_spec.rb +++ b/modules/auth_plugins/spec/requests/auth_plugins_spec.rb @@ -64,6 +64,9 @@ RSpec.describe OpenProject::Plugins::AuthPlugin, with_ee: %i[sso_auth_providers] without_partial_double_verification do allow(dummy_engine_klass).to receive(:engine_name).and_return("foobar") allow(dummy_engine_klass).to receive(:initializer) { |_, &block| app.instance_eval(&block) } + allow(dummy_engine_klass).to receive_message_chain(:config, :to_prepare) { |_, &block| # rubocop:disable RSpec/MessageChain + block.call + } end end @@ -97,7 +100,11 @@ RSpec.describe OpenProject::Plugins::AuthPlugin, with_ee: %i[sso_auth_providers] expect(strategies.keys.to_a).to eq %i[strategy_a strategy_b] end - it "registers register each strategy (i.e. middleware) only once" do + it "persists all strategies in the database" do + expect(PluginAuthProvider.pluck(:slug)).to contain_exactly("a1", "a2", "b1", "c1") + end + + it "registers each strategy (i.e. middleware) only once" do expect(middlewares.size).to eq 2 expect(middlewares).to eq %i[strategy_a strategy_b] end diff --git a/modules/auth_saml/lib/open_project/auth_saml/engine.rb b/modules/auth_saml/lib/open_project/auth_saml/engine.rb index 8c0c7295891..4cd75a89c9f 100644 --- a/modules/auth_saml/lib/open_project/auth_saml/engine.rb +++ b/modules/auth_saml/lib/open_project/auth_saml/engine.rb @@ -37,7 +37,7 @@ module OpenProject auth_provider-saml.png ) - register_auth_providers do + register_auth_providers(persist: false) do strategy :saml do OpenProject::AuthSaml.configuration.values.map do |h| # Remember saml session values when logging in user diff --git a/modules/openid_connect/lib/open_project/openid_connect/engine.rb b/modules/openid_connect/lib/open_project/openid_connect/engine.rb index a7d9b37a20f..ed2968c0c11 100644 --- a/modules/openid_connect/lib/open_project/openid_connect/engine.rb +++ b/modules/openid_connect/lib/open_project/openid_connect/engine.rb @@ -60,7 +60,7 @@ module OpenProject::OpenIDConnect class_inflection_override("openid_connect" => "OpenIDConnect") - register_auth_providers do + register_auth_providers(persist: false) do OmniAuth::OpenIDConnect::Providers.configure custom_options: %i[ display_name? icon?