diff --git a/app/models/plugin_auth_provider.rb b/app/models/plugin_auth_provider.rb index 603a45d3d0a..e6c1db1367b 100644 --- a/app/models/plugin_auth_provider.rb +++ b/app/models/plugin_auth_provider.rb @@ -30,16 +30,28 @@ class PluginAuthProvider < AuthProvider class << self - def create_for_plugin(config) + def register_for_plugin(config) slug = config[:name] - display_name = config[:display_name] || slug + display_name = (config[:display_name] || slug).to_s - find_or_create_by!(slug:) do |provider| - provider.available = false - provider.display_name = display_name.to_s - provider.creator = User.system + registry[slug] = display_name + end + + def create_all_registered + registry.each do |slug, display_name| + find_or_create_by!(slug:) do |provider| + provider.available = false + provider.display_name = display_name + provider.creator = User.system + end end end + + private + + def registry + @registry ||= {} + end end def human_type diff --git a/app/seeders/basic_data/plugin_auth_provider_seeder.rb b/app/seeders/basic_data/plugin_auth_provider_seeder.rb new file mode 100644 index 00000000000..846c5c7667d --- /dev/null +++ b/app/seeders/basic_data/plugin_auth_provider_seeder.rb @@ -0,0 +1,37 @@ +# 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 BasicData + class PluginAuthProviderSeeder < Seeder + def seed_data! + PluginAuthProvider.create_all_registered + end + end +end diff --git a/app/seeders/standard/basic_data_seeder.rb b/app/seeders/standard/basic_data_seeder.rb index a0f02771f4b..08dc6cff6a5 100644 --- a/app/seeders/standard/basic_data_seeder.rb +++ b/app/seeders/standard/basic_data_seeder.rb @@ -37,6 +37,7 @@ module Standard ::BasicData::TimeEntryActivitySeeder, ::BasicData::ColorSeeder, ::BasicData::ColorSchemeSeeder, + ::BasicData::PluginAuthProviderSeeder, ::BasicData::ProjectPhaseColorSeeder, ::BasicData::ProjectPhaseDefinitionSeeder, ::BasicData::StatusSeeder, diff --git a/db/migrate/20250512114003_move_users_identity_url_to_user_auth_provider_links.rb b/db/migrate/20250512114003_move_users_identity_url_to_user_auth_provider_links.rb index 67a50695465..a5c094277d3 100644 --- a/db/migrate/20250512114003_move_users_identity_url_to_user_auth_provider_links.rb +++ b/db/migrate/20250512114003_move_users_identity_url_to_user_auth_provider_links.rb @@ -39,34 +39,6 @@ class MoveUsersIdentityUrlToUserAuthProviderLinks < ActiveRecord::Migration[8.0] t.index %i[auth_provider_id external_id], unique: true end - reversible do |direction| - direction.up do - users_data = execute(<<-SQL.squish).values - SELECT id, identity_url FROM users WHERE type = 'User' AND NOT (identity_url = '' OR identity_url IS NULL); - SQL - auth_providers_data = execute(<<-SQL.squish).values - SELECT id, slug FROM auth_providers - SQL - - insert_data = users_data.filter_map do |user_id, identity_url| - slug, external_id = identity_url.split(":", 2) - next if slug.blank? || external_id.blank? - - auth_provider_id = auth_providers_data.find { |_, auth_provider_slug| auth_provider_slug == slug }&.first - next if auth_provider_id.blank? - - "(#{user_id}, #{auth_provider_id}, '#{external_id}', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" - end.join(",\n") - - if insert_data.present? - execute(<<-SQL.squish).values - INSERT INTO user_auth_provider_links (user_id, auth_provider_id, external_id, created_at, updated_at) - VALUES #{insert_data} - ON CONFLICT DO NOTHING - RETURNING id - SQL - end - end - end + # Code to migrate data into the new table was moved into 20250804133700 after adding a fix end end diff --git a/db/migrate/20250804133700_migrate_auth_provider_urls_again.rb b/db/migrate/20250804133700_migrate_auth_provider_urls_again.rb new file mode 100644 index 00000000000..55ad828aeba --- /dev/null +++ b/db/migrate/20250804133700_migrate_auth_provider_urls_again.rb @@ -0,0 +1,65 @@ +# 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 MigrateAuthProviderUrlsAgain < ActiveRecord::Migration[8.0] + def change + reversible do |direction| + direction.up do + PluginAuthProvider.create_all_registered + + users_data = execute(<<-SQL.squish).values + SELECT id, identity_url FROM users WHERE type = 'User' AND NOT (identity_url = '' OR identity_url IS NULL); + SQL + auth_providers_data = execute(<<-SQL.squish).values + SELECT id, slug FROM auth_providers + SQL + + insert_data = users_data.filter_map do |user_id, identity_url| + slug, external_id = identity_url.split(":", 2) + next if slug.blank? || external_id.blank? + + auth_provider_id, = auth_providers_data.find { |_, auth_provider_slug| auth_provider_slug == slug } + next if auth_provider_id.blank? + + "(#{user_id}, #{auth_provider_id}, '#{external_id}', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" + end.join(",\n") + + if insert_data.present? + execute(<<-SQL.squish).values + INSERT INTO user_auth_provider_links (user_id, auth_provider_id, external_id, created_at, updated_at) + VALUES #{insert_data} + ON CONFLICT DO NOTHING + RETURNING id + SQL + end + end + end + end +end 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 a852fd642c9..75446d0c4a0 100644 --- a/modules/auth_plugins/lib/open_project/plugins/auth_plugin.rb +++ b/modules/auth_plugins/lib/open_project/plugins/auth_plugin.rb @@ -44,7 +44,7 @@ module OpenProject::Plugins if persist builder.provider_callbacks.each do |callback| callback.call.each do |config| - PluginAuthProvider.create_for_plugin(config) + PluginAuthProvider.register_for_plugin(config) 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 dc9819230cc..c8792ff640f 100644 --- a/modules/auth_plugins/spec/requests/auth_plugins_spec.rb +++ b/modules/auth_plugins/spec/requests/auth_plugins_spec.rb @@ -100,7 +100,9 @@ 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 "persists all strategies in the database" do + it "marks all strategies for persisting in the database" do + expect(PluginAuthProvider.pluck(:slug)).to eq([]) + PluginAuthProvider.create_all_registered expect(PluginAuthProvider.pluck(:slug)).to contain_exactly("a1", "a2", "b1", "c1") end diff --git a/spec/migrations/migrate_auth_provider_urls_again_spec.rb b/spec/migrations/migrate_auth_provider_urls_again_spec.rb new file mode 100644 index 00000000000..660a0f4d93e --- /dev/null +++ b/spec/migrations/migrate_auth_provider_urls_again_spec.rb @@ -0,0 +1,110 @@ +# 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 Rails.root.join("db/migrate/20250804133700_migrate_auth_provider_urls_again.rb") + +RSpec.describe MigrateAuthProviderUrlsAgain, type: :model do + subject(:execute_migration) { ActiveRecord::Migration.suppress_messages { described_class.new.change } } + + let!(:auth_provider) { create(:oidc_provider, slug: "oidc-my-slug") } + let!(:user) { create(:user) } + let(:identity_url) { "oidc-my-slug:abc1337" } + + before do + # side-stepping ActiveRecord here, because we let AR ignore the identity_url column already + User.connection.execute("UPDATE users SET identity_url = '#{identity_url}' WHERE users.id = #{user.id}") + end + + it "succeeds" do + expect { execute_migration }.not_to raise_error + end + + it "creates a user auth provider link" do + expect { execute_migration }.to change(UserAuthProviderLink, :count).from(0).to(1) + expect(UserAuthProviderLink.first.attributes.symbolize_keys).to include( + external_id: "abc1337", auth_provider_id: auth_provider.id, user_id: user.id + ) + end + + context "when no auth provider matches the slug" do + let!(:auth_provider) { create(:oidc_provider, slug: "oidc-other-slug") } + + it "succeeds" do + expect { execute_migration }.not_to raise_error + end + + it "creates no user auth provider link" do + expect { execute_migration }.not_to change(UserAuthProviderLink, :count) + end + end + + context "when the user auth provider link already exists" do + before do + UserAuthProviderLink.create!(principal: user, auth_provider:, external_id: "abc1337") + end + + it "succeeds" do + expect { execute_migration }.not_to raise_error + end + + it "creates no additional user auth provider link" do + expect { execute_migration }.not_to change(UserAuthProviderLink, :count) + end + end + + context "when there are PluginAuthProviders to be persisted" do + let(:pending_providers) { { my_slug: "A display name" } } + let(:identity_url) { "my_slug:1337-7331" } + + before do + # Hooking into implementation of PluginAuthProvider.create_all_registered to cover that as well in this test case + allow(PluginAuthProvider).to receive(:registry).and_return(pending_providers) + end + + it "succeeds" do + expect { execute_migration }.not_to raise_error + end + + it "persists them" do + expect { execute_migration }.to change(PluginAuthProvider, :count).from(0).to(1) + expect(PluginAuthProvider.first.attributes.symbolize_keys).to include( + slug: "my_slug", display_name: "A display name", available: false + ) + end + + it "is able to create user auth provider links for them" do + expect { execute_migration }.to change(UserAuthProviderLink, :count).from(0).to(1) + expect(UserAuthProviderLink.first.attributes.symbolize_keys).to include( + external_id: "1337-7331", auth_provider_id: PluginAuthProvider.first.id, user_id: user.id + ) + end + end +end