Merge pull request #17221 from opf/fix/saml-limit-self-registration

Respect self-registration in saml

modules/auth_saml/app/components/saml/providers/sections/show_component.html.erb

@@ -33,6 +33,7 @@
                 tag: :a,
                 scheme: :invisible,
                 href: edit_saml_provider_path(provider, edit_state: @target_state),
+                test_selector:  "saml_provider_#{@target_state}_edit",
                 data: { turbo: true, turbo_stream: true },
                 aria: { label: I18n.t(disabled ? :label_show : :label_edit) }
               )

modules/auth_saml/app/controllers/saml/providers_controller.rb

@@ -92,7 +92,7 @@ def update
         successful_save_response
       else
         @provider = call.result
-        render action: :edit
+        render action: :edit, status: :unprocessable_entity
       end
     end
 
@@ -178,7 +178,7 @@ def create_params
     def update_params
       params
         .require(:saml_provider)
-        .permit(:display_name, *Saml::Provider.stored_attributes[:options])
+        .permit(:display_name, :limit_self_registration, *Saml::Provider.stored_attributes[:options])
     end
 
     def find_provider

modules/auth_saml/app/services/saml/configuration_mapper.rb

@@ -39,8 +39,9 @@ def call!
       {
         "options" => options,
         "slug" => options.delete("name"),
+        "limit_self_registration" => ActiveModel::Type::Boolean.new.cast(options.delete("limit_self_registration")),
         "display_name" => options.delete("display_name") || "SAML"
-      }
+      }.compact
     end
 
     private

modules/auth_saml/spec/features/administration/saml_crud_spec.rb

@@ -57,6 +57,7 @@
       fill_in "Identity provider login endpoint", with: "https://example.com/sso"
       fill_in "Identity provider logout endpoint", with: "https://example.com/slo"
       fill_in "Public certificate of identity provider", with: CertificateHelper.valid_certificate.to_pem
+      check "Limit self registration"
 
       click_link_or_button "Continue"
 
@@ -68,11 +69,11 @@
       click_link_or_button "Continue"
 
       # Mapping form
-      fill_in "Mapping for: Username", with: "login\nmail", fill_options: { clear: :backspace }
-      fill_in "Mapping for: Email", with: "mail", fill_options: { clear: :backspace }
-      fill_in "Mapping for: First name", with: "myName", fill_options: { clear: :backspace }
-      fill_in "Mapping for: Last name", with: "myLastName", fill_options: { clear: :backspace }
-      fill_in "Mapping for: Internal user id", with: "uid", fill_options: { clear: :backspace }
+      fill_in "Mapping for: Username", with: "login\nmail"
+      fill_in "Mapping for: Email", with: "mail"
+      fill_in "Mapping for: First name", with: "myName"
+      fill_in "Mapping for: Last name", with: "myLastName"
+      fill_in "Mapping for: Internal user id", with: "uid"
 
       click_link_or_button "Continue"
 
@@ -105,6 +106,7 @@
       expect(provider.mapping_lastname).to eq "myLastName"
       expect(provider.mapping_uid).to eq "uid"
       expect(provider.authn_requests_signed).to be true
+      expect(provider.limit_self_registration).to be true
 
       click_link_or_button "Delete"
       # Confirm the deletion
@@ -175,6 +177,19 @@
 
         expect(page).to have_text "Display name has already been taken."
       end
+
+      it "can toggle limit_self_registration (Regression #59370)" do
+        visit "/admin/saml/providers"
+        click_link_or_button "My provider"
+
+        page.find_test_selector("saml_provider_configuration_edit").click
+        check "Limit self registration"
+        click_link_or_button "Update"
+        wait_for_network_idle
+
+        provider.reload
+        expect(provider.limit_self_registration).to be true
+      end
     end
   end
 

modules/auth_saml/spec/services/saml/configuration_mapper_spec.rb

@@ -50,6 +50,34 @@
     end
   end
 
+  describe "limit_self_registration" do
+    subject { result["limit_self_registration"] }
+
+    context "when provided as string" do
+      let(:configuration) { { limit_self_registration: "1" } }
+
+      it { is_expected.to be(true) }
+    end
+
+    context "when provided as false boolean" do
+      let(:configuration) { { limit_self_registration: false } }
+
+      it { is_expected.to be(false) }
+    end
+
+    context "when provided as true boolean" do
+      let(:configuration) { { limit_self_registration: true } }
+
+      it { is_expected.to be(true) }
+    end
+
+    context "when not provided" do
+      let(:configuration) { {} }
+
+      it { is_expected.to be_nil }
+    end
+  end
+
   describe "slug" do
     subject { result["slug"] }