Merge pull request #23392 from opf/fix/ldap-seeder-aliases

Fix LDAP seeder not using env aliases and underscores

app/seeders/env_data/ldap_seeder.rb

@@ -29,9 +29,20 @@
 # See COPYRIGHT and LICENSE files for more details.
 module EnvData
   class LdapSeeder < Seeder
+    KNOWN_CONNECTION_KEYS = %w[
+      host port security tls_verify tls_certificate sync_users
+      filter basedn binduser bindpassword
+      login_mapping firstname_mapping lastname_mapping mail_mapping admin_mapping
+      groupfilter
+    ].freeze
+
+    KNOWN_FILTER_KEYS = %w[base filter sync_users group_attribute].freeze
+
     def seed_data!
       print_status "    ↳ Creating LDAP connection" do
         Setting.seed_ldap.each do |name, options|
+          validate_options!(name, options)
+
           ldap = LdapAuthSource.find_or_initialize_by(name:)
 
           print_ldap_status(ldap)
@@ -51,6 +62,34 @@ module EnvData
 
     private
 
+    def validate_options!(name, options)
+      check_unknown_keys!(options, KNOWN_CONNECTION_KEYS,
+                          scope: "LDAP connection '#{name}'")
+
+      filters = options["groupfilter"]
+      return if filters.blank?
+
+      filters.each do |filter_name, filter_options|
+        check_unknown_keys!(filter_options, KNOWN_FILTER_KEYS,
+                            scope: "LDAP group filter '#{filter_name}' (connection '#{name}')")
+      end
+    end
+
+    def check_unknown_keys!(options, known_keys, scope:)
+      unknown = options.keys - known_keys
+      return if unknown.empty?
+
+      raise <<~MSG.strip
+        #{scope}: unknown configuration key(s): #{unknown.map { |k| env_form(k) }.join(', ')}.
+        Accepted keys: #{known_keys.map { |k| env_form(k) }.join(', ')}.
+        Note: in environment variable names, single underscores split path segments and double underscores encode a literal underscore (e.g. LOGIN__MAPPING, not LOGIN_MAPPING).
+      MSG
+    end
+
+    def env_form(key)
+      key.gsub("_", "__").upcase
+    end
+
     # rubocop:disable Metrics/AbcSize
     def upsert_settings(ldap, options)
       ldap.host = options["host"]

docs/installation-and-operations/configuration/README.md

@@ -229,43 +229,65 @@ The connection can be set with the following options. Please note that "EXAMPLE"
 
 The name of the LDAP connection is derived from the ENV key behind `SEED_LDAP_`, so you need to take care to use only valid characters. If you need to place an underscore, use a double underscore to encode it e.g., `my__ldap`.
 
-The following options are possible
+#### Naming rule for option keys
+
+The same encoding applies to **option keys** (the part of the env var after the connection name):
+
+- A single underscore (`_`) separates path segments.
+- A double underscore (`__`) encodes a literal underscore inside a single key.
+
+For example, the attribute mapping for the login attribute must be passed as `LOGIN__MAPPING`. Writing `LOGIN_MAPPING` would create a nested `login.mapping` hash and would have been silently wrong in OpenProject versions earlier than 17.5.
+
+Starting with OpenProject 17.5 the seeder validates the keys it sees and raises an error listing any unknown key, so typos no longer go unnoticed. Use exactly the keys shown below.
+
+#### Full example
+
+The example below shows every supported option for a connection named `EXAMPLE`.
 
 ```shell
 # Host name of the connection
-OPENPROJECT_SEED_LDAP_EXAMPLE_HOST="localhost"
+OPENPROJECT_SEED_LDAP_EXAMPLE_HOST="ldap.example.com"
+
 # Port of the connection
 OPENPROJECT_SEED_LDAP_EXAMPLE_PORT="389"
-# LDAP security options. One of the following
+
+# LDAP security mode. One of:
 #   plain_ldap: Unencrypted connection, no TLS/SSL
-# simple_tls: Using deprecated LDAPS/SSL (often in combination with port 636)
-# start_tls: LDAPv3 start_tls call using standard unencrypted port (e.g., 389) before upgrading connection
+#   simple_tls: Deprecated LDAPS/SSL (often combined with port 636)
+#   start_tls:  LDAPv3 STARTTLS on the standard unencrypted port (e.g., 389)
 OPENPROJECT_SEED_LDAP_EXAMPLE_SECURITY="start_tls"
-# Whether to verify the certificate/chain of the LDAP connection. true/false (True by default)
+
+# Whether to verify the LDAP server's certificate chain. true/false (true by default).
 OPENPROJECT_SEED_LDAP_EXAMPLE_TLS__VERIFY="true"
-# Optionally, provide a certificate of the connection
+
+# Optionally pin a server certificate (PEM-encoded).
 OPENPROJECT_SEED_LDAP_EXAMPLE_TLS__CERTIFICATE="-----BEGIN CERTIFICATE-----\nMII....\n-----END CERTIFICATE-----"
-# The admin LDAP bind account with read access
+
+# Bind DN (the LDAP account used for read access during search/bind).
 OPENPROJECT_SEED_LDAP_EXAMPLE_BINDUSER="uid=admin,ou=system"
-# Password for the bind account
+
+# Password for the bind account.
 OPENPROJECT_SEED_LDAP_EXAMPLE_BINDPASSWORD="secret"
-# BASE DN of the connection
+
+# Base DN of the directory subtree used for user search.
 OPENPROJECT_SEED_LDAP_EXAMPLE_BASEDN="dc=example,dc=com"
-# Optional filter string to restrict which users may log in to OpenProject
-# (relevant when for automatic creation of users is active)
+
+# Optional LDAP filter restricting which users may log in to OpenProject
+# (used when automatic user creation is active).
 OPENPROJECT_SEED_LDAP_EXAMPLE_FILTER="(uid=*)"
-# Whether to create found and matching users automatically when they log in
+
+# Whether to create matching users on the fly when they log in for the first time.
 OPENPROJECT_SEED_LDAP_EXAMPLE_SYNC__USERS="true"
-# Attribute mapping for the OpenProject login attribute
+
+# Attribute mappings: which LDAP attribute should populate each OpenProject field.
+# Remember the double underscore in these keys.
 OPENPROJECT_SEED_LDAP_EXAMPLE_LOGIN__MAPPING="uid"
-# Attribute mapping for the OpenProject first name attribute
 OPENPROJECT_SEED_LDAP_EXAMPLE_FIRSTNAME__MAPPING="givenName"
-# Attribute mapping for the OpenProject last name attribute
 OPENPROJECT_SEED_LDAP_EXAMPLE_LASTNAME__MAPPING="sn"
-# Attribute mapping for the OpenProject mail attribute
 OPENPROJECT_SEED_LDAP_EXAMPLE_MAIL__MAPPING="mail"
-# Attribute mapping for the OpenProject admin attribute
-# Leave empty or remove to not derive admin status from an attribute
+
+# Optional: derive admin status from an LDAP attribute. Leave empty or remove
+# to keep admin status managed manually in OpenProject.
 OPENPROJECT_SEED_LDAP_EXAMPLE_ADMIN__MAPPING=""
 ```
 

spec/seeders/env_data/ldap_seeder_spec.rb

@@ -248,4 +248,51 @@ RSpec.describe EnvData::LdapSeeder do
       expect(names).to contain_exactly("another")
     end
   end
+
+  context "when an unknown key is provided",
+          :settings_reset,
+          with_env: {
+            OPENPROJECT_SEED_LDAP_FOO_HOST: "localhost",
+            OPENPROJECT_SEED_LDAP_FOO_PORT: "12389",
+            OPENPROJECT_SEED_LDAP_FOO_SECURITY: "plain_ldap",
+            OPENPROJECT_SEED_LDAP_FOO_BINDUSER: "uid=admin,ou=system",
+            OPENPROJECT_SEED_LDAP_FOO_BINDPASSWORD: "secret",
+            OPENPROJECT_SEED_LDAP_FOO_BASEDN: "dc=example,dc=com",
+            OPENPROJECT_SEED_LDAP_FOO_LOGIN__MAPPING: "uid",
+            OPENPROJECT_SEED_LDAP_FOO_FIRSTNAME__MAPPING: "givenName",
+            OPENPROJECT_SEED_LDAP_FOO_LASTNAME__MAPPING: "sn",
+            OPENPROJECT_SEED_LDAP_FOO_MAIL__MAPPING: "mail",
+            OPENPROJECT_SEED_LDAP_FOO_NONSENSE: "boom"
+          } do
+    it "raises an error naming the unknown key without creating the record" do
+      reset(:seed_ldap)
+
+      expect { seeder.seed! }
+        .to raise_error(/LDAP connection 'foo'.*NONSENSE/m)
+        .and not_change(LdapAuthSource, :count)
+    end
+  end
+
+  context "when a typo'd nested key would be silently swallowed",
+          :settings_reset,
+          with_env: {
+            OPENPROJECT_SEED_LDAP_FOO_HOST: "localhost",
+            OPENPROJECT_SEED_LDAP_FOO_PORT: "12389",
+            OPENPROJECT_SEED_LDAP_FOO_SECURITY: "plain_ldap",
+            OPENPROJECT_SEED_LDAP_FOO_BINDUSER: "uid=admin,ou=system",
+            OPENPROJECT_SEED_LDAP_FOO_BINDPASSWORD: "secret",
+            OPENPROJECT_SEED_LDAP_FOO_BASEDN: "dc=example,dc=com",
+            OPENPROJECT_SEED_LDAP_FOO_LOGIN_MAPPING: "uid",
+            OPENPROJECT_SEED_LDAP_FOO_FIRSTNAME__MAPPING: "givenName",
+            OPENPROJECT_SEED_LDAP_FOO_LASTNAME__MAPPING: "sn",
+            OPENPROJECT_SEED_LDAP_FOO_MAIL__MAPPING: "mail"
+          } do
+    it "raises rather than silently producing a nil login attribute" do
+      reset(:seed_ldap)
+
+      expect { seeder.seed! }
+        .to raise_error(/LDAP connection 'foo'.*LOGIN/m)
+        .and not_change(LdapAuthSource, :count)
+    end
+  end
 end

spec/support/components/work_packages/table_configuration_modal.rb

@@ -80,8 +80,10 @@ module Components
       end
 
       def save
+        retry_block do
           find('[data-test-selector="spot-modal-wp-table-configuration-save-button"]').click
           expect_closed
+        end
 
         if using_cuprite?
           wait_for_reload