Fix creation of plugin auth providers

The previous implementation would have failed when
starting with an empty database and trying to migrate it.

We now ensure that the database exists, by running the creation
of plugin auth providers in the seeders. Since there is also a migration
that's supposed to convert old identity_urls to auth provider links
and that migration needs the auth providers already, it does the same
as the seeder.
This commit is contained in:
Jan Sandbrink
2025-08-04 10:57:33 +02:00
parent e1e907dde8
commit c00fefe8ce
8 changed files with 236 additions and 37 deletions
+18 -6
View File
@@ -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
@@ -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
@@ -37,6 +37,7 @@ module Standard
::BasicData::TimeEntryActivitySeeder,
::BasicData::ColorSeeder,
::BasicData::ColorSchemeSeeder,
::BasicData::PluginAuthProviderSeeder,
::BasicData::ProjectPhaseColorSeeder,
::BasicData::ProjectPhaseDefinitionSeeder,
::BasicData::StatusSeeder,
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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