allow to limit self registration for openid connect providers

app/services/users/register_user_service.rb

@@ -104,7 +104,7 @@ module Users
     # Try to register a user with an existsing omniauth connection
     # bypassing regular account registration restrictions
     def register_omniauth_user
-      return if user.identity_url.blank?
+      return if skip_omniauth_user?
 
       user.activate
 
@@ -113,6 +113,10 @@ module Users
       end
     end
 
+    def skip_omniauth_user?
+      user.identity_url.blank?
+    end
+
     def register_by_email_activation
       return unless Setting::SelfRegistration.by_email?
 

modules/auth_plugins/lib/open_project/auth_plugins/engine.rb

@@ -38,6 +38,8 @@ module OpenProject::AuthPlugins
              author_url: 'https://www.openproject.org',
              bundled: true
 
+    patch_with_namespace :Users, :RegisterUserService
+
     config.to_prepare do
       OpenProject::AuthPlugins::Hooks
     end

modules/auth_plugins/lib/open_project/auth_plugins/patches.rb

@@ -0,0 +1,30 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 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 OpenProject::AuthPlugins::Patches
+end

modules/auth_plugins/lib/open_project/auth_plugins/patches/register_user_service_patch.rb

@@ -0,0 +1,45 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 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 OpenProject::AuthPlugins::Patches::RegisterUserServicePatch
+  def self.included(base) # :nodoc:
+    base.prepend InstanceMethods
+  end
+
+  module InstanceMethods
+    def skip_omniauth_user?
+      super || limit_self_registration?(user)
+    end
+
+    def limit_self_registration?(user)
+      provider = user.authentication_provider.downcase
+
+      OpenProject::Plugins::AuthPlugin.limit_self_registration? provider:
+    end
+  end
+end

modules/auth_plugins/lib/open_project/plugins/auth_plugin.rb

@@ -92,6 +92,15 @@ module OpenProject::Plugins
       [camelization, name].compact.first.underscore.to_sym
     end
 
+    ##
+    # Indicates whether or not self registration should be limited for the provider
+    # with the given name.
+    #
+    # @param provider [String] Name of the provider
+    def self.limit_self_registration?(provider:)
+      Hash(find_provider_by_name(provider))[:limit_self_registration]
+    end
+
     def self.warn_unavailable(name)
       RequestStore.fetch("warn_unavailable_auth_#{name}") do
         Rails.logger.warn { "OmniAuth SSO strategy #{name} is only available for Enterprise Editions." }

modules/openid_connect/app/controllers/openid_connect/providers_controller.rb

@@ -62,11 +62,11 @@ module OpenIDConnect
     end
 
     def create_params
-      params.require(:openid_connect_provider).permit(:name, :display_name, :identifier, :secret)
+      params.require(:openid_connect_provider).permit(:name, :display_name, :identifier, :secret, :limit_self_registration)
     end
 
     def update_params
-      params.require(:openid_connect_provider).permit(:display_name, :identifier, :secret)
+      params.require(:openid_connect_provider).permit(:display_name, :identifier, :secret, :limit_self_registration)
     end
 
     def find_provider

modules/openid_connect/app/models/openid_connect/provider.rb

@@ -21,14 +21,27 @@ module OpenIDConnect
     delegate :scope, to: :omniauth_provider, allow_nil: true
     delegate :to_h, to: :omniauth_provider, allow_nil: false
 
+    ##
+    # Controls whether or not self registration shall be limited for this provider.
+    #
+    # See also:
+    #   - OpenProject::Plugins::AuthPlugin.limit_self_registration?
+    #   - OpenProject::AuthPlugins::Patches::RegisterUserServicePatch
+    attr_reader :limit_self_registration
+
     def initialize(omniauth_provider)
       @omniauth_provider = omniauth_provider
       @errors = ActiveModel::Errors.new(self)
       @display_name = omniauth_provider.to_h[:display_name]
+      @limit_self_registration = initial_value_for_limit_self_registration
     end
 
     def self.initialize_with(params)
-      new(NewProvider.new(params))
+      do_limit = params[:limit_self_registration]
+
+      new(NewProvider.new(params.except(:limit_self_registration))).tap do |p|
+        p.limit_self_registration = String(do_limit).to_bool unless do_limit.nil?
+      end
     end
 
     def new_record?
@@ -39,6 +52,24 @@ module OpenIDConnect
       omniauth_provider.is_a?(OmniAuth::OpenIDConnect::Provider)
     end
 
+    def limit_self_registration?
+      @limit_self_registration
+    end
+
+    def limit_self_registration=(value)
+      @limit_self_registration = value
+    end
+
+    def to_h
+      return {} if omniauth_provider.nil?
+
+      omniauth_provider.to_h.merge(limit_self_registration: limit_self_registration?)
+    end
+
+    def limit_self_registration_default
+      name == "google" # limit by default only for Google since anyone can sign in
+    end
+
     def id
       return nil unless persisted?
 
@@ -46,33 +77,64 @@ module OpenIDConnect
     end
 
     def valid?
-      @errors.add(:name, :invalid) unless ALLOWED_TYPES.include?(name)
+      @errors.add(:name, :invalid) unless type_allowed?(name)
       @errors.add(:identifier, :blank) if identifier.blank?
       @errors.add(:secret, :blank) if secret.blank?
       @errors.none?
     end
 
+    ##
+    # Checks if the provider with the given name is of an allowed type.
+    #
+    # Types can be followed by a period and arbitrary names to add several
+    # providers of the same type. E.g. 'azure', 'azure.dep1', 'azure.dep2'.
+    def type_allowed?(name)
+      ALLOWED_TYPES.any? { |allowed| name =~ /\A#{allowed}(\..+)?\Z/ }
+    end
+
     def save
       return false unless valid?
 
-      config = Setting.plugin_openproject_openid_connect || Hash.new
-      config["providers"] ||= Hash.new
-      config["providers"][name] = omniauth_provider.to_h.stringify_keys
-      Setting.plugin_openproject_openid_connect = config
+      Setting.plugin_openproject_openid_connect = setting_with_provider
+
       true
     end
 
     def destroy
-      config = Setting.plugin_openproject_openid_connect
-      config["providers"] ||= {}
-      config["providers"].delete(name)
-      Setting.plugin_openproject_openid_connect = config
+      Setting.plugin_openproject_openid_connect = setting_without_provider
+
       true
     end
 
+    def setting_with_provider
+      setting.deep_merge "providers" => { name => to_h.stringify_keys }
+    end
+
+    def setting_without_provider
+      setting.tap do |s|
+        s["providers"].delete name
+      end
+    end
+
+    def setting
+      Hash(Setting.plugin_openproject_openid_connect).tap do |h|
+        h["providers"] ||= Hash.new
+      end
+    end
+
     # https://api.rubyonrails.org/classes/ActiveModel/Errors.html
     def read_attribute_for_validation(attr)
       send(attr)
     end
+
+    private
+
+    def initial_value_for_limit_self_registration
+      if omniauth_provider.configuration&.has_key? :limit_self_registration
+        omniauth_provider.configuration[:limit_self_registration]
+      else
+        limit_self_registration_default
+      end
+    end
   end
 end

modules/openid_connect/app/views/openid_connect/providers/_form.html.erb

@@ -22,4 +22,11 @@
   <div class="form--field -required">
     <%= f.text_field :secret, required: true, container_class: '-middle' %>
   </div>
+
+  <div class="form--field">
+    <%= f.check_box :limit_self_registration, required: false, container_class: '-middle' %>
+    <div class="form--field-instructions">
+      <%= I18n.t('openid_connect.setting_instructions.limit_self_registration') %>
+    </div>
+  </div>
 </fieldset>

modules/openid_connect/config/locales/de.yml

@@ -10,6 +10,7 @@ de:
         identifier: Identifier
         secret: Secret
         scope: Scope
+        limit_self_registration: Selbstregistrierung einschränken
   openid_connect:
     menu_title: OpenID-Provider
     providers:
@@ -20,3 +21,7 @@ de:
       singular: OpenID-Provider
       upsale:
         description: Use existing OpenID credentials with OpenProject for easier access and interoperability with a range of other providers.
+    setting_instructions:
+      limit_self_registration: >
+        Wenn diese Option aktiv ist, können sich neue Nutzer mit diesem OpenID-Provider nur registrieren,
+        wenn die Selbstregistrierungs-Einstellung es erlaubt.

modules/openid_connect/config/locales/en.yml

@@ -10,6 +10,7 @@ en:
         identifier: Identifier
         secret: Secret
         scope: Scope
+        limit_self_registration: Limit self registration
   openid_connect:
     menu_title: OpenID providers
     providers:
@@ -18,3 +19,6 @@ en:
       no_results_table: No providers have been defined yet.
       plural: OpenID providers
       singular: OpenID provider
+    setting_instructions:
+      limit_self_registration: >
+        If enabled users can only register using this provider if the self registration setting allows for it.

modules/openid_connect/spec/controllers/providers_controller_spec.rb

@@ -105,11 +105,40 @@ describe OpenIDConnect::ProvidersController do
     end
 
     describe '#create' do
-      it 'is successful if valid params' do
-        post :create, params: { openid_connect_provider: valid_params }
-        expect(flash[:notice]).to eq(I18n.t(:notice_successful_create))
-        expect(Setting.plugin_openproject_openid_connect["providers"]).to have_key("azure")
-        expect(response).to be_redirect
+      context 'with valid params' do
+        let(:params) { { openid_connect_provider: valid_params } }
+
+        before do
+          post :create, params:
+        end
+
+        it 'is successful' do
+          expect(flash[:notice]).to eq(I18n.t(:notice_successful_create))
+          expect(Setting.plugin_openproject_openid_connect["providers"]).to have_key("azure")
+          expect(response).to be_redirect
+        end
+
+        context 'with limit_self_registration checked' do
+          let(:params) do
+            { openid_connect_provider: valid_params.merge(limit_self_registration: 1) }
+          end
+
+          it 'sets the setting' do
+            expect(OpenProject::Plugins::AuthPlugin)
+              .to be_limit_self_registration provider: valid_params[:name]
+          end
+        end
+
+        context 'with limit_self_registration unchecked' do
+          let(:params) do
+            { openid_connect_provider: valid_params.merge(limit_self_registration: 0) }
+          end
+
+          it 'does not set the setting' do
+            expect(OpenProject::Plugins::AuthPlugin)
+              .not_to be_limit_self_registration provider: valid_params[:name]
+          end
+        end
       end
 
       it 'renders an error if invalid params' do
@@ -130,6 +159,39 @@ describe OpenIDConnect::ProvidersController do
           expect(assigns[:provider]).to be_present
           expect(response).to render_template 'edit'
         end
+
+        context(
+          'with limit_self_registration set',
+          with_settings: {
+            plugin_openproject_openid_connect: {
+              "providers" => {
+                "azure" => {
+                  "identifier" => "IDENTIFIER",
+                  "secret" => "SECRET",
+                  "limit_self_registration" => true
+                }
+              }
+            }
+          }
+        ) do
+          before do
+            get :edit, params: { id: 'azure' }
+          end
+
+          it 'shows limit_self_registration as checked' do
+            expect(assigns[:provider]).to be_limit_self_registration
+          end
+        end
+
+        context 'with limit_self_registration not set' do
+          before do
+            get :edit, params: { id: 'azure' }
+          end
+
+          it 'shows limit_self_registration as unchecked' do
+            expect(assigns[:provider]).not_to be_limit_self_registration
+          end
+        end
       end
 
       context 'when not found' do

modules/openid_connect/spec/models/provider_spec.rb

@@ -0,0 +1,96 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 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'
+
+describe OpenIDConnect::Provider do
+  let(:provider) do
+    described_class.initialize_with name: "azure", identifier: "id", secret: "secret"
+  end
+
+  def auth_plugin
+    OpenProject::Plugins::AuthPlugin
+  end
+
+  describe 'limit_self_registration' do
+    before do
+      # required so that the auth plugin sees any providers (ee feature)
+      allow(EnterpriseToken).to receive(:show_banners?).and_return false
+    end
+
+    context 'with no limited providers' do
+      it "shows the provider as unlimited" do
+        expect(auth_plugin).not_to be_limit_self_registration provider: provider.name
+      end
+
+      context 'when set to true' do
+        before do
+          provider.limit_self_registration = true
+        end
+
+        it "saving the provider makes it limited" do
+          provider.save
+
+          expect(auth_plugin).to be_limit_self_registration provider: provider.name
+        end
+      end
+
+      context 'when set to false' do
+        before do
+          provider.limit_self_registration = false
+        end
+
+        it "saving the provider does nothing" do
+          provider.save
+
+          expect(auth_plugin).not_to be_limit_self_registration provider: provider.name
+        end
+      end
+    end
+
+    context(
+      'with a limited provider',
+      with_settings: {
+        plugin_openproject_openid_connect: {
+          "providers" => {
+            "azure" => {
+              "name" => "azure",
+              "identifier" => "id",
+              "secret" => "secret",
+              "limit_self_registration" => true
+            }
+          }
+        }
+      }
+    ) do
+      it "shows the provider as limited" do
+        expect(auth_plugin).to be_limit_self_registration provider: provider.name
+      end
+    end
+  end
+end

spec/services/users/register_user_service_spec.rb

@@ -76,6 +76,48 @@ describe Users::RegisterUserService do
     end
   end
 
+  describe '#register_omniauth_user' do
+    let(:user) { User.new(status: Principal.statuses[:registered], identity_url: 'azure:1234') }
+    let(:instance) { described_class.new(user) }
+    let(:call) { instance.call }
+
+    before do
+      allow(user).to receive(:activate)
+      allow(user).to receive(:save).and_return true
+
+      # required so that the azure provider is visible (ee feature)
+      allow(EnterpriseToken).to receive(:show_banners?).and_return false
+
+      with_all_registration_options do |_type|
+        call
+      end
+    end
+
+    it 'tries to activate that user regardless of settings' do
+      expect(call).to be_success
+      expect(call.result).to eq user
+      expect(call.message).to eq I18n.t(:notice_account_registered_and_logged_in)
+    end
+
+    context(
+      'with limit_self_registration enabled and self_registration disabled',
+      with_settings: {
+        self_registration: 0,
+        plugin_openproject_openid_connect: {
+          providers: {
+            azure: { identifier: "foo", secret: "bar", limit_self_registration: true }
+          }
+        }
+      }
+    ) do
+      it 'fails to activate due to disabled self registration' do
+        expect(call).not_to be_success
+        expect(call.result).to eq user
+        expect(call.message).to eq I18n.t('account.error_self_registration_disabled')
+      end
+    end
+  end
+
   describe '#ensure_registration_allowed!' do
     it 'returns an error for disabled' do
       allow(Setting).to receive(:self_registration).and_return(0)