Add migration spec

modules/openid_connect/app/services/openid_connect/sync_service.rb

@@ -33,21 +33,31 @@ class SyncService
     def initialize(name, configuration)
       @name = name
       configuration[:name] = name
-      @configuration = ::OpenIDConnect::ConfigurationMapper.new(configuration).call!
+      @configuration = configuration
     end
 
-    def call # rubocop:disable Metrics/AbcSize
+    def call
+      mapped_configuration = ::OpenIDConnect::ConfigurationMapper.new(@configuration).call!
       provider = ::OpenIDConnect::Provider.find_by(slug: name)
+
+      service_call(provider, mapped_configuration)
+    rescue StandardError => e
+      ServiceResult.failure(message: e.message)
+    end
+
+    private
+
+    def service_call(provider, configuration) # rubocop:disable Metrics/AbcSize
       if provider
         ::OpenIDConnect::Providers::UpdateService
           .new(model: provider, user: User.system)
-          .call(@configuration)
+          .call(configuration)
           .on_success { |call| call.message = "Successfully updated OpenID provider #{name}." }
           .on_failure { |call| call.message = "Failed to update OpenID provider: #{call.message}" }
       else
         ::OpenIDConnect::Providers::CreateService
           .new(user: User.system)
-          .call(@configuration)
+          .call(configuration)
           .on_success { |call| call.message = "Successfully created OpenID provider #{name}." }
           .on_failure { |call| call.message = "Failed to create OpenID provider: #{call.message}" }
       end

modules/openid_connect/spec/migration/migrate_oidc_settings_to_providers_spec.rb

@@ -27,42 +27,120 @@
 #++
 
 require "spec_helper"
-require_relative "openid_connect_spec_helpers"
 
-require Rails.root.join('db/migrate/20190106184413_remove_email_from_users.rb')
-RSpec.describe RemoveEmailFromUsers do
-  let(:migrations_paths) { ActiveRecord::Migrator.migrations_paths }
-  let(:migrations) { ActiveRecord::MigrationContext.new(migrations_paths).migrations }
-  let(:previous_version) { 20181204143322 }
-  let(:current_version) { 20190106184413 }
-  subject { ActiveRecord::Migrator.new(:up, migrations, current_version).migrate }
+require Rails.root.join("modules/openid_connect/db/migrate/20240829140616_migrate_oidc_settings_to_providers.rb")
+RSpec.describe MigrateOidcSettingsToProviders, type: :model do
+  # Define a custom class,
+  # Class.new doesn't work here as STI uses .constantize
+  # rubocop:disable Lint/ConstantDefinitionInBlock,RSpec/LeakyConstantDeclaration
+  class TestOpenIDConnectProvider < OpenIDConnect::Provider
+    self.table_name = "test_openid_connect_provider"
+  end
+  # rubocop:enable Lint/ConstantDefinitionInBlock,RSpec/LeakyConstantDeclaration
+
+  subject { described_class.new.up }
+
+  let(:test_provider_table_name) { "test_openid_connect_provider" }
+
+  # Mock the create service so we can test with the test provider
+  before do
+    allow_any_instance_of(OpenIDConnect::Providers::CreateService) # rubocop:disable RSpec/AnyInstance
+      .to receive(:instance_class).and_return(TestOpenIDConnectProvider)
+  end
+
   around do |example|
-    # Silence migrations output in specs report.
     ActiveRecord::Migration.suppress_messages do
-      # Migrate back to the previous version
-      ActiveRecord::Migrator.new(:down, migrations, ActiveRecord::SchemaMigration, previous_version).migrate
+      ActiveRecord::Migration.create_table(test_provider_table_name) do |t|
+        t.string :type, null: false
+        t.string :display_name, null: false, index: { unique: true }
+        t.string :slug, null: false, index: { unique: true }
+        t.boolean :available, null: false, default: true
+        t.boolean :limit_self_registration, null: false, default: false
+        t.jsonb :options, default: {}, null: false
+        t.references :creator, null: false, index: true, foreign_key: { to_table: :users }
+
+        t.timestamps
+      end
 
-      # If other tests using User table ran before this one, Rails has
-      # stored information about table's columns and we need to reset those
-      # since the migration changed the database structure.
-      User.reset_column_information
       example.run
+    ensure
+      ActiveRecord::Migration.drop_table(test_provider_table_name)
+    end
+  end
+
+  context "when no provider present",
+          with_settings: { plugin_openproject_openid_connect: nil } do
+    it "does nothing" do
+      allow(OpenIDConnect::SyncService).to receive(:new)
+
+      subject
+
+      expect(OpenIDConnect::SyncService).not_to have_received(:new)
+    end
+  end
 
-      # Re-update column information after the migration has been executed
-      # again in the example. This will make user attributes cache
-      # ready for other tests.
-      User.reset_column_information
+  context "when a provider present, but invalid",
+          with_settings: {
+            plugin_openproject_openid_connect: {
+              providers: {
+                keycloak: {
+                  display_name: "Keycloak",
+                  token_endpoint: "wat?"
+                }
+              }
+            }
+          } do
+    it "raises an error" do
+      expect { subject }.to raise_error do |error|
+        expect(error.message).to include "Failed to create or update OpenID provider keycloak from previous settings format"
+        expect(error.message).to include "Provided token_endpoint 'wat?' needs to be http(s) URL or path starting with a slash."
+      end
     end
   end
-  context 'when there are users with email' do
-    let(:user_with_email) { create(:user, email: '[email protected]') }
-    it 'creates an Email associated with the same user' do
-      expect { subject }
-        .to change { Email.all.size }
-              .from(0)
-              .to(1)
-      expect(Email.first.user).to eq user_with_email
-      expect(Email.first.value).to eq '[email protected]'
+
+  context "when a provider correctly provided",
+          with_settings: {
+            plugin_openproject_openid_connect: {
+              providers: {
+                keycloak: {
+                  display_name: "Keycloak",
+                  host: "localhost",
+                  port: "8080",
+                  scheme: "http",
+                  identifier: "http://localhost:3000",
+                  secret: "IVl6GxxujAQ3mt6thAXKxyYYvmyRr8jw",
+                  issuer: "http://localhost:8080/realms/test",
+                  authorization_endpoint: "/realms/test/protocol/openid-connect/auth",
+                  token_endpoint: "/realms/test/protocol/openid-connect/token",
+                  userinfo_endpoint: "/realms/test/protocol/openid-connect/userinfo",
+                  end_session_endpoint: "http://localhost:8080/realms/test/protocol/openid-connect/logout",
+                  post_logout_redirect_uri: "http://localhost:3000",
+                  limit_self_registration: true,
+                  attribute_map: { login: "foo", admin: "isAdmin" }
+                }
+              }
+            }
+          } do
+    it "migrates correctly" do # rubocop:disable RSpec/MultipleExpectations
+      expect { subject }.to change(TestOpenIDConnectProvider, :count).by(1)
+
+      provider = TestOpenIDConnectProvider.last
+      expect(provider.display_name).to eq "Keycloak"
+      expect(provider.host).to eq "localhost"
+      expect(provider.port).to eq "8080"
+      expect(provider.scheme).to eq "http"
+      expect(provider.client_id).to eq "http://localhost:3000"
+      expect(provider.client_secret).to eq "IVl6GxxujAQ3mt6thAXKxyYYvmyRr8jw"
+      expect(provider.issuer).to eq "http://localhost:8080/realms/test"
+      expect(provider.authorization_endpoint).to eq "http://localhost:8080/realms/test/protocol/openid-connect/auth"
+      expect(provider.token_endpoint).to eq "http://localhost:8080/realms/test/protocol/openid-connect/token"
+      expect(provider.userinfo_endpoint).to eq "http://localhost:8080/realms/test/protocol/openid-connect/userinfo"
+      expect(provider.end_session_endpoint).to eq "http://localhost:8080/realms/test/protocol/openid-connect/logout"
+      expect(provider.post_logout_redirect_uri).to eq "http://localhost:3000"
+      expect(provider.limit_self_registration).to be true
+      expect(provider.mapping_login).to eq "foo"
+      expect(provider.mapping_admin).to eq "isAdmin"
+      expect(provider.mapping_email).to be_blank
     end
   end
 end