Skip to content

Commit

Permalink
Relax validation of openldap_database's suffix parameter
Browse files Browse the repository at this point in the history
The validation of the `suffix` parameter assumed conformance with
RFC2247, but this is not a requirement and cause trouble to some users.

A full validation of the suffix syntax to check if it conforms to
RFC2253 is probably overkill, so for now, we just relax the used regexp
to allow valid values which where previously rejected.

Some valid DN syntax are still rejected, as supporting them would
require to replace the regexp with a parser.  Add them as known-bad
examples in the test suite (pending tests).

Fixes #391
Fixes #396
  • Loading branch information
smortex committed Jan 5, 2024
1 parent 51c6fd1 commit 85772ef
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
8 changes: 1 addition & 7 deletions lib/puppet/type/openldap_database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,7 @@
newparam(:suffix, namevar: true) do
desc 'The default namevar.'
validate do |value|
raise ArgumentError, 'Invalid value' unless [
/\Acn=config\z/,
/\Acn=monitor\z/,
%r{\A(dc|o)=[[:alnum:].-]+(,(dc|o)=[[:alnum:].-]+)*\z},
].any? do |pattern|
pattern.match?(value)
end
raise ArgumentError, 'Invalid value' unless value.match?(%r{\A[[:alnum:]]+=[^,;]+([,;][[:alnum:]]+=[^,;]+)*\z})
end
end

Expand Down
11 changes: 9 additions & 2 deletions spec/unit/puppet/type/openldap_database_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,21 @@

it 'does not invalid suffixes' do
expect { described_class.new(name: 'foo bar') }.to raise_error(Puppet::Error, %r{Invalid value})
expect { described_class.new(name: 'cn=admin,dc=example,dc=com') }.to raise_error(Puppet::Error, %r{Invalid value})
expect { described_class.new(name: 'dc=example, dc=com') }.to raise_error(Puppet::Error, %r{Invalid value})
expect { described_class.new(name: 'cn=foo,bar') }.to raise_error(Puppet::Error, %r{Invalid value})
expect { described_class.new(name: 'foo,cn=bar') }.to raise_error(Puppet::Error, %r{Invalid value})
expect { described_class.new(name: 'cn=foo,,cn=bar') }.to raise_error(Puppet::Error, %r{Invalid value})
end

it 'allows valid suffix' do
expect { described_class.new(name: 'dc=example,dc=com') }.not_to raise_error
expect { described_class.new(name: 'dc=foo;dc=bar') }.not_to raise_error
expect { described_class.new(name: 'cn=config') }.not_to raise_error
end

it 'allows more valid suffix', skip: 'These are valid DNs, but we need a proper parser to match them, not a regexp' do

Check warning on line 31 in spec/unit/puppet/type/openldap_database_spec.rb

View workflow job for this annotation

GitHub Actions / Puppet / 8 (Ruby 3.2)

Puppet::Type::Openldap_database namevar validation allows more valid suffix Skipped: These are valid DNs, but we need a proper parser to match them, not a regexp

Check warning on line 31 in spec/unit/puppet/type/openldap_database_spec.rb

View workflow job for this annotation

GitHub Actions / Puppet / 7 (Ruby 2.7)

Puppet::Type::Openldap_database namevar validation allows more valid suffix Skipped: These are valid DNs, but we need a proper parser to match them, not a regexp
expect { described_class.new(name: 'OU=Sales+CN=J. Smith,O=Widget Inc.,C=US') }.not_to raise_error
expect { described_class.new(name: 'CN=R. Smith,O=Big Company\\, Inc.,C=US') }.not_to raise_error
end
end

describe 'when validating attributes' do
Expand Down

0 comments on commit 85772ef

Please sign in to comment.