diff --git a/.envrc b/.envrc index d84c0f8..1164e64 100644 --- a/.envrc +++ b/.envrc @@ -21,8 +21,8 @@ export K_SOUP_COV_DO=true # Means you want code coverage export K_SOUP_COV_COMMAND_NAME="Test Coverage" # Available formats are html, xml, rcov, lcov, json, tty export K_SOUP_COV_FORMATTERS="html,xml,rcov,lcov,json,tty" -export K_SOUP_COV_MIN_BRANCH=70 # Means you want to enforce X% branch coverage -export K_SOUP_COV_MIN_LINE=93 # Means you want to enforce X% line coverage +export K_SOUP_COV_MIN_BRANCH=78 # Means you want to enforce X% branch coverage +export K_SOUP_COV_MIN_LINE=98 # Means you want to enforce X% line coverage export K_SOUP_COV_MIN_HARD=true # Means you want the build to fail if the coverage thresholds are not met export K_SOUP_COV_MULTI_FORMATTERS=true export K_SOUP_COV_OPEN_BIN= # Means don't try to open coverage results in browser diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 5a9b42e..1d7963a 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -6,8 +6,8 @@ permissions: id-token: write env: - K_SOUP_COV_MIN_BRANCH: 70 - K_SOUP_COV_MIN_LINE: 93 + K_SOUP_COV_MIN_BRANCH: 78 + K_SOUP_COV_MIN_LINE: 98 K_SOUP_COV_MIN_HARD: true K_SOUP_COV_FORMATTERS: "xml,rcov,lcov,tty" K_SOUP_COV_DO: true @@ -115,7 +115,7 @@ jobs: hide_complexity: true indicators: true output: both - thresholds: '93 70' + thresholds: '98 78' continue-on-error: ${{ matrix.experimental != 'false' }} - name: Add Coverage PR Comment diff --git a/.gitignore b/.gitignore index fc62abc..afc4cd4 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ +.project +.tags + # Build Artifacts /pkg/ /tmp/ diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fddd2b0..20d10f1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -13,7 +13,7 @@ # - template: Security/SAST.gitlab-ci.yml default: - image: ruby + image: "ruby:${RUBY_VERSION}" variables: BUNDLE_INSTALL_FLAGS: "--quiet --jobs=$(nproc) --retry=3" @@ -39,7 +39,6 @@ workflow: - if: '$CI_COMMIT_TAG' .test_template-current: &test_definition-current - image: ruby:${RUBY_VERSION} stage: test script: # || true so we don't fail here, because it'll probably work even if the gem update fails @@ -67,7 +66,6 @@ workflow: - vendor/ruby .test_template-legacy: &test_definition-legacy - image: ruby:${RUBY_VERSION} stage: test script: # RUBYGEMS_VERSION because we support EOL Ruby still... diff --git a/.rubocop_gradual.lock b/.rubocop_gradual.lock index 489de8c..b4ed25a 100644 --- a/.rubocop_gradual.lock +++ b/.rubocop_gradual.lock @@ -1,9 +1,9 @@ { - "lib/omniauth-ldap/adaptor.rb:2352927785": [ - [36, 7, 413, "Style/ClassMethodsDefinitions: Use `class << self` to define a class method.", 105664470], - [86, 17, 3, "Style/AndOr: Use `&&` instead of `and`.", 193409806], - [86, 30, 3, "Style/AndOr: Use `&&` instead of `and`.", 193409806], - [86, 37, 1, "Lint/AssignmentInCondition: Wrap assignment in parentheses if intentional", 177560] + "lib/omniauth-ldap/adaptor.rb:715274645": [ + [64, 7, 413, "Style/ClassMethodsDefinitions: Use `class << self` to define a class method.", 105664470], + [114, 17, 3, "Style/AndOr: Use `&&` instead of `and`.", 193409806], + [114, 30, 3, "Style/AndOr: Use `&&` instead of `and`.", 193409806], + [114, 37, 1, "Lint/AssignmentInCondition: Wrap assignment in parentheses if intentional", 177560] ], "spec/integration/middleware_spec.rb:4062046892": [ [3, 16, 39, "RSpec/DescribeClass: The first argument to describe should be the class or module being tested.", 638096201], @@ -15,29 +15,30 @@ [4, 3, 12, "RSpec/BeforeAfterAll: Beware of using `before(:all)` as it may cause state to leak between tests. If you are using `rspec-rails`, and `use_transactional_fixtures` is enabled, then records created in `before(:all)` are not automatically rolled back.", 86334566], [70, 16, 5, "RSpec/ExpectActual: Provide the actual value you are testing to `expect(...)`.", 237881235] ], - "spec/omniauth-ldap/adaptor_spec.rb:3624298807": [ - [72, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], - [73, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], - [74, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], - [80, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], - [81, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], - [82, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310] + "spec/omniauth-ldap/adaptor_spec.rb:2715031579": [ + [3, 1, 38, "RSpec/SpecFilePathFormat: Spec path should end with `omni_auth/ldap/adaptor*_spec.rb`.", 1973618936], + [206, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], + [207, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], + [208, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], + [214, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], + [215, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310], + [216, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310] ], - "spec/omniauth/adaptor_spec.rb:3492754784": [ + "spec/omniauth/adaptor_spec.rb:1168013709": [ [3, 1, 38, "RSpec/SpecFilePathFormat: Spec path should end with `omni_auth/ldap/adaptor*_spec.rb`.", 1973618936], - [42, 7, 38, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 3627954156], - [43, 7, 38, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 3627954156], - [80, 7, 48, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 2759780562] + [46, 7, 38, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 3627954156], + [47, 7, 38, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 3627954156], + [84, 7, 48, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 2759780562] ], - "spec/omniauth/strategies/ldap_spec.rb:3760791626": [ - [13, 3, 54, "RSpec/LeakyConstantDeclaration: Stub class constant instead of declaring explicitly.", 2419068710], - [76, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1130140517], - [101, 17, 28, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3444838747], - [110, 17, 23, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1584148894], - [121, 17, 32, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1515076977], - [129, 19, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694], - [141, 17, 56, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2413495789], - [156, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3182939526], - [189, 15, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694] + "spec/omniauth/strategies/ldap_spec.rb:86189447": [ + [14, 3, 54, "RSpec/LeakyConstantDeclaration: Stub class constant instead of declaring explicitly.", 2419068710], + [90, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1130140517], + [145, 17, 28, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3444838747], + [154, 17, 23, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1584148894], + [165, 17, 32, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1515076977], + [174, 19, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694], + [187, 17, 56, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2413495789], + [202, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3182939526], + [235, 15, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694] ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index a35f990..069ba29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,9 @@ [![SemVer 2.0.0][📌semver-img]][📌semver] [![Keep-A-Changelog 1.0.0][📗keep-changelog-img]][📗keep-changelog] -All notable changes to this project will be documented in this file. +Since version v2.3.1, all notable changes to this project will be documented in this file. + +This changelog lists the releases of the original omniauth-ldap, and the GitLab forked versions, up until v2.3.0. The format is based on [Keep a Changelog][📗keep-changelog], and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html), @@ -20,12 +22,158 @@ Please file a bug if you notice a violation of semantic versioning. ### Added +- Added RBS types +- Upgraded RSpec tests to v3 syntax +- Improved code coverage to 98% lines and 78% branches +- Added integration tests with a complete Roda-based demo app for specs +- Well tested support for all versions of OmniAuth >= v1 and Rack >= v1 via appraisals + ### Changed +- Make support for Ruby v2.0 explicit +- Make support for OmniAuth v1+ explicit +- Make support for Rack v1+ explicit +- Modernize codebase to use more recent Ruby syntax (upgrade from Ruby v1 to v2 syntax) and conventions + ### Deprecated ### Removed ### Fixed -### Security \ No newline at end of file +- Prevent key duplication in symbolize_hash_keys + +### Security + +## [2.3.0-gl] (gitlab fork) - 2025-08-20 + +- TAG: [v2.3.0][2.3.0t-gl] (gitlab) + +## [2.2.0-gl] (gitlab fork) - 2022-06-24 + +- TAG: [v2.2.0][2.2.0t-gl] (gitlab) + +## [2.1.1-gl] (gitlab fork) - 2019-02-22 + +- TAG: [v2.1.1][2.1.1t-gl] (gitlab) + +### Added + +- Add a String check to `tls_options` sanitization to allow other objects + +## [2.1.0-gl] (gitlab fork) - 2018-06-18 + +- TAG: [v2.1.0][2.1.0t-gl] (gitlab) + +### Added + +- Expose `:tls_options` SSL configuration option. + +### Deprecated + +- Deprecate :ca_file, :ssl_version + +## [2.0.4-gl] (gitlab fork) - 2017-08-10 + +- TAG: [v2.0.4][2.0.4t-gl] (gitlab) + +- Improve log message when invalid credentials are used + +## 2.0.3 (gitlab fork) - 2017-07-20 + +- Protects against wrong request method call to callback + +## [2.0.2-gl] (gitlab fork) - 2017-06-13 + +- TAG: [v2.0.2][2.0.2t-gl] (gitlab) + +## [2.0.1-gl] (gitlab fork) - 2017-06-09 + +- TAG: [v2.0.1][2.0.1t-gl] (gitlab) + +## [2.0.0-gl] (gitlab fork) - 2017-06-07 + +- TAG: [v2.0.0][2.0.0t-gl] (gitlab) + +## [2.0.0] (intridea) - 2018-01-09 + +- TAG: [v2.0.0][2.0.0t] (github) + +## [1.2.1-gl] (gitlab fork) - 2015-03-17 + +- TAG: [v1.2.1][1.2.1t-gl] (gitlab) + +## [1.2.0-gl] (gitlab fork) - 2014-10-29 + +- TAG: [v1.2.0][1.2.0t-gl] (gitlab) + +## [1.1.0-gl] (gitlab fork) - 2014-09-08 + +- TAG: [v1.1.0][1.1.0t-gl] (gitlab) + +## [1.0.5-gl] - 2016-02-17 + +- TAG: [v1.0.5][1.0.5t-gl] (gitlab fork, gem not released) +- TAG: [v1.0.5][1.0.5t] (github) + +## 1.0.4 + +- released 2014-02-03 (intridea) +- released 2013-11-13 (gitlab fork) + +## 1.0.3 + +- released 2013-01-23 (intridea) +- released 2013-06-13 (gitlab fork) + +## [1.0.2-gl] + +- TAG: [v1.0.2][1.0.2t-gl] (gitlab) - released 2012-12-30 +- TAG: [v1.0.2][1.0.2t] (github) - released 2011-12-17 + +## 1.0.1 - 2011-11-02 + +## [1.0.0-gl] - 2011-11-02 + +- TAG: [v1.0.0][1.0.0t-gl] (gitlab) +- TAG: [v1.0.0][1.0.0t] (github) + +[2.3.0-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v2.2.0...v2.3.0 +[2.3.0t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/2.3.0 +[2.2.0-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v2.1.1...v2.2.0 +[2.2.0t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/2.2.0 +[2.1.1-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v2.1.0...v2.1.1 +[2.1.1t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/2.1.1 +[2.1.0-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v2.0.4...v2.1.0 +[2.1.0t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/2.1.0 +[2.0.4-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v2.0.2...v2.0.4 +[2.0.4t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/2.0.4 +[//]: # ( There is no tag for v2.0.3 on GitLab) +[2.0.2-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v2.0.1...v2.0.2 +[2.0.2t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/2.0.2 +[2.0.1-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v2.0.0...v2.0.1 +[2.0.1t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/2.0.1 +[2.0.0-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v1.2.1...v2.0.0 +[2.0.0t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/2.0.0 +[2.0.0]: https://github.com/omniauth/omniauth-ldap/compare/v1.0.5...v2.0.0 +[2.0.0t]: https://github.com/omniauth/omniauth-ldap/releases/tag/v2.0.0 +[1.2.1-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v1.2.0...v1.2.1 +[1.2.1t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/1.2.1 +[1.2.0-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v1.1.0...v1.2.0 +[1.2.0t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/1.2.0 +[1.1.0-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v1.0.2...v1.1.0 +[1.1.0t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/1.1.0 +[1.0.5-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v1.0.2...v1.0.5 +[1.0.5t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/1.0.5 +[1.0.5]: https://github.com/omniauth/omniauth-ldap/compare/v1.0.2...v1.0.5 +[1.0.5t]: https://github.com/omniauth/omniauth-ldap/releases/tag/v1.0.5 +[//]: # ( There are no tags for v1.0.3, v1.0.4 on GitHub, or GitLab) +[1.0.2-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/v1.0.1...v1.0.2 +[1.0.2t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/1.0.2 +[1.0.2]: https://github.com/omniauth/omniauth-ldap/compare/v1.0.1...v1.0.2 +[1.0.2t]: https://github.com/omniauth/omniauth-ldap/releases/tag/v1.0.2 +[//]: # ( There are no tags for v1.0.1 on GitHub, or GitLab) +[1.0.0-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/compare/5656da80d4193e0d0584f44bac493a87695e580f...v1.0.0 +[1.0.0t-gl]: https://gitlab.com/gitlab-org/ruby/gems/omniauth-ldap/-/tags/1.0.0 +[1.0.0]: https://github.com/omniauth/omniauth-ldap/compare/5656da80d4193e0d0584f44bac493a87695e580f...v1.0.0 +[1.0.0t]: https://github.com/omniauth/omniauth-ldap/releases/tag/v1.0.0 diff --git a/Gemfile.lock b/Gemfile.lock index 094b0d1..56ee66d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -13,12 +13,12 @@ GIT PATH remote: . specs: - omniauth-ldap (2.0.0) - net-ldap (~> 0.16) - omniauth (>= 1) - pyu-ruby-sasl (~> 0.0.3.3) - rack (>= 1) - rubyntlm (~> 0.6.2) + omniauth-ldap (2.3.0) + net-ldap (~> 0.16, < 1) + omniauth (>= 1, < 3) + pyu-ruby-sasl (>= 0.0.3.3, < 0.1) + rack (>= 1, < 4) + rubyntlm (~> 0.6.2, < 1) version_gem (~> 1.1, >= 1.1.9) GEM @@ -157,7 +157,6 @@ GEM net-ldap (0.20.0) base64 ostruct - nkf (0.2.0) nokogiri (1.18.10-x86_64-linux-gnu) racc (~> 1.4) notiffany (0.1.3) @@ -393,7 +392,6 @@ DEPENDENCIES kramdown-parser-gfm (~> 1.1) logger (~> 1.7) mutex_m (~> 0.2) - nkf omniauth-ldap! rack-test (~> 2.2) rake (~> 13.0) diff --git a/README.md b/README.md index e5fc4fe..4c8f9f6 100644 --- a/README.md +++ b/README.md @@ -57,12 +57,16 @@ use OmniAuth::Strategies::LDAP, title: "My LDAP", host: "10.101.10.1", port: 389, - method: :plain, + encryption: :plain, base: "dc=intridea,dc=com", uid: "sAMAccountName", name_proc: proc { |name| name.gsub(/@.*$/, "") }, bind_dn: "default_bind_dn", - password: "password" + password: "password", + tls_options: { + ssl_version: "TLSv1_2", + ciphers: ["AES-128-CBC", "AES-128-CBC-HMAC-SHA1", "AES-128-CBC-HMAC-SHA256"], + } # Or, alternatively: # use OmniAuth::Strategies::LDAP, filter: '(&(uid=%{username})(memberOf=cn=myapp-users,ou=groups,dc=example,dc=com))' ``` @@ -93,20 +97,6 @@ Compatible with MRI Ruby 2.0+, and concordant releases of JRuby, and TruffleRuby |------------------------------------------------|--------------------------------------------------------| | 👟 Check it out! | ✨ [github.com/appraisal-rb/appraisal2][💎appraisal2] ✨ | -#### Ruby 3.4 - -nkf/kconv has been part of Ruby since long ago. -Eventually it became a standard gem, but was changed to a bundled gem in Ruby 3.4. -In general, kconv and iconv have been superseded since Ruby 1.9 by the built-in -encoding support provided by String#encode, String#force_encoding, and similar methods. -But this gem has not yet been updated to remove its dependency on nkf/kconv. - -As a result of all this you should add `nkf` to your Gemfile if you are using Ruby 3.4 or later. - -```ruby -gem "nkf", "~> 0.1" -``` - ### Enterprise Support [![Tidelift](https://tidelift.com/badges/package/rubygems/omniauth-ldap)](https://tidelift.com/subscription/pkg/rubygems-omniauth-ldap?utm_source=rubygems-omniauth-ldap&utm_medium=referral&utm_campaign=readme) Available as part of the Tidelift Subscription. @@ -642,8 +632,8 @@ Thanks for RTFM. ☺️ [📌changelog]: CHANGELOG.md [📗keep-changelog]: https://keepachangelog.com/en/1.0.0/ [📗keep-changelog-img]: https://img.shields.io/badge/keep--a--changelog-1.0.0-34495e.svg?style=flat -[📌gitmoji]:https://gitmoji.dev -[📌gitmoji-img]:https://img.shields.io/badge/gitmoji_commits-%20%F0%9F%98%9C%20%F0%9F%98%8D-34495e.svg?style=flat-square +[📌gitmoji]: https://gitmoji.dev +[📌gitmoji-img]: https://img.shields.io/badge/gitmoji_commits-%20%F0%9F%98%9C%20%F0%9F%98%8D-34495e.svg?style=flat-square [🧮kloc]: https://www.youtube.com/watch?v=dQw4w9WgXcQ [🧮kloc-img]: https://img.shields.io/badge/KLOC-4.076-FFDD67.svg?style=for-the-badge&logo=YouTube&logoColor=blue [🔐security]: SECURITY.md diff --git a/gemfiles/modular/optional.gemfile b/gemfiles/modular/optional.gemfile index b0232fe..8526e8f 100644 --- a/gemfiles/modular/optional.gemfile +++ b/gemfiles/modular/optional.gemfile @@ -3,9 +3,3 @@ # Required for kettle-pre-release # URL parsing with Unicode support (falls back to URI if not available) gem "addressable", ">= 2.8", "< 3" # ruby >= 2.2 - -# nkf/kconv has been part of Ruby since long ago. -# Eventually it became a standard gem, but was changed to a bundled gem in Ruby 3.4. -# In general, kconv and iconv have been superseded since Ruby 1.9 by the built-in -# encoding support provided by String#encode, String#force_encoding, and similar methods. -gem "nkf" # ruby >= 2.3 diff --git a/lib/omniauth-ldap/adaptor.rb b/lib/omniauth-ldap/adaptor.rb index ed41b23..23cb13a 100644 --- a/lib/omniauth-ldap/adaptor.rb +++ b/lib/omniauth-ldap/adaptor.rb @@ -1,10 +1,6 @@ -# this code borrowed pieces from activeldap and net-ldap +# frozen_string_literal: true -# nkf/kconv has been part of Ruby since long ago. -# Eventually it became a standard gem, but was changed to a bundled gem in Ruby 3.4. -# In general, kconv and iconv have been superseded since Ruby 1.9 by the built-in -# encoding support provided by String#encode, String#force_encoding, and similar methods. -require "nkf" +# this code borrowed pieces from activeldap and net-ldap # External Gems require "net/ldap" @@ -20,19 +16,51 @@ class ConfigurationError < StandardError; end class AuthenticationError < StandardError; end class ConnectionError < StandardError; end - VALID_ADAPTER_CONFIGURATION_KEYS = [:host, :port, :method, :bind_dn, :password, :try_sasl, :sasl_mechanisms, :uid, :base, :allow_anonymous, :filter] + VALID_ADAPTER_CONFIGURATION_KEYS = [ + :hosts, + :host, + :port, + :encryption, + :disable_verify_certificates, + :bind_dn, + :password, + :try_sasl, + :sasl_mechanisms, + :uid, + :base, + :allow_anonymous, + :filter, + :tls_options, + + # Deprecated + :method, + :ca_file, + :ssl_version, + ] # A list of needed keys. Possible alternatives are specified using sub-lists. - MUST_HAVE_KEYS = [:host, :port, :method, [:uid, :filter], :base] + MUST_HAVE_KEYS = [ + :base, + [:encryption, :method], # :method is deprecated + [:hosts, :host], + [:hosts, :port], + [:uid, :filter], + ] + + ENCRYPTION_METHOD = { + simple_tls: :simple_tls, + start_tls: :start_tls, + plain: nil, - METHOD = { + # Deprecated. This mapping aimed to be user-friendly, but only caused + # confusion. Better to pass through the actual `Net::LDAP` encryption type. ssl: :simple_tls, tls: :start_tls, - plain: nil, } attr_accessor :bind_dn, :password attr_reader :connection, :uid, :base, :auth, :filter + def self.validate(configuration = {}) message = [] MUST_HAVE_KEYS.each do |names| @@ -53,11 +81,12 @@ def initialize(configuration = {}) VALID_ADAPTER_CONFIGURATION_KEYS.each do |name| instance_variable_set("@#{name}", @configuration[name]) end - method = ensure_method(@method) config = { + base: @base, + hosts: @hosts, host: @host, port: @port, - base: @base, + encryption: encryption_options, } @bind_method = if @try_sasl :sasl @@ -72,7 +101,6 @@ def initialize(configuration = {}) password: @password, } config[:auth] = @auth - config[:encryption] = method @connection = Net::LDAP.new(config) end @@ -103,14 +131,48 @@ def bind_as(args = {}) private - def ensure_method(method) + def encryption_options + translated_method = translate_method + return unless translated_method + + { + method: translated_method, + tls_options: tls_options(translated_method), + } + end + + def translate_method + method = @encryption || @method method ||= "plain" normalized_method = method.to_s.downcase.to_sym - return METHOD[normalized_method] if METHOD.has_key?(normalized_method) - available_methods = METHOD.keys.collect { |m| m.inspect }.join(", ") - format = "%s is not one of the available connect methods: %s" - raise ConfigurationError, format % [method.inspect, available_methods] + unless ENCRYPTION_METHOD.has_key?(normalized_method) + available_methods = ENCRYPTION_METHOD.keys.collect { |m| m.inspect }.join(", ") + format = "%s is not one of the available connect methods: %s" + raise ConfigurationError, format % [method.inspect, available_methods] + end + + ENCRYPTION_METHOD[normalized_method] + end + + def tls_options(translated_method) + return {} if translated_method.nil? # (plain) + + options = default_options + + if @tls_options + # Prevent blank config values from overwriting SSL defaults + configured_options = sanitize_hash_values(@tls_options) + configured_options = symbolize_hash_keys(configured_options) + + options.merge!(configured_options) + end + + # Retain backward compatibility until deprecated configs are removed. + options[:ca_file] = @ca_file if @ca_file + options[:ssl_version] = @ssl_version if @ssl_version + + options end def sasl_auths(options = {}) @@ -157,6 +219,37 @@ def sasl_bind_setup_gss_spnego(options) } [Net::NTLM::Message::Type1.new.serialize, nego] end + + private + + def default_options + if @disable_verify_certificates + # It is important to explicitly set verify_mode for two reasons: + # 1. The behavior of OpenSSL is undefined when verify_mode is not set. + # 2. The net-ldap gem implementation verifies the certificate hostname + # unless verify_mode is set to VERIFY_NONE. + {verify_mode: OpenSSL::SSL::VERIFY_NONE} + else + OpenSSL::SSL::SSLContext::DEFAULT_PARAMS.dup + end + end + + # Removes keys that have blank values + # + # This gem may not always be in the context of Rails so we + # do this rather than `.blank?`. + def sanitize_hash_values(hash) + hash.delete_if do |_, value| + value.nil? || + (value.is_a?(String) && value !~ /\S/) + end + end + + def symbolize_hash_keys(hash) + hash.each_with_object({}) do |(key, value), result| + result[key.to_sym] = value + end + end end end end diff --git a/lib/omniauth-ldap/version.rb b/lib/omniauth-ldap/version.rb index e210899..f4f6047 100644 --- a/lib/omniauth-ldap/version.rb +++ b/lib/omniauth-ldap/version.rb @@ -1,7 +1,7 @@ module OmniAuth module LDAP module Version - VERSION = "2.0.0" + VERSION = "2.3.0" end VERSION = Version::VERSION # Make VERSION available in traditional way end diff --git a/lib/omniauth/strategies/ldap.rb b/lib/omniauth/strategies/ldap.rb index 40e36c9..ddadbd6 100644 --- a/lib/omniauth/strategies/ldap.rb +++ b/lib/omniauth/strategies/ldap.rb @@ -1,4 +1,5 @@ require "omniauth" +require "omniauth/version" module OmniAuth module Strategies @@ -6,6 +7,8 @@ class LDAP OMNIAUTH_GTE_V2 = Gem::Version.new(OmniAuth::VERSION) >= Gem::Version.new("2.0.0") include OmniAuth::Strategy + InvalidCredentialsError = Class.new(StandardError) + CONFIG = { "name" => "cn", "first_name" => "givenName", @@ -31,6 +34,9 @@ class LDAP end option :port, 389 option :method, :plain + option :disable_verify_certificates, false + option :ca_file, nil + option :ssl_version, nil # use OpenSSL default if nil option :uid, "sAMAccountName" option :name_proc, lambda { |n| n } @@ -59,10 +65,14 @@ def request_phase def callback_phase @adaptor = OmniAuth::LDAP::Adaptor.new(@options) + return fail!(:invalid_request_method) unless valid_request_method? return fail!(:missing_credentials) if missing_credentials? begin @ldap_user_info = @adaptor.bind_as(filter: filter(@adaptor), size: 1, password: request.params["password"]) - return fail!(:invalid_credentials) unless @ldap_user_info + + unless @ldap_user_info + return fail!(:invalid_credentials, InvalidCredentialsError.new("Invalid credentials for #{request.params["username"]}")) + end @user_info = self.class.map_user(CONFIG, @ldap_user_info) super @@ -73,9 +83,10 @@ def callback_phase def filter(adaptor) if adaptor.filter && !adaptor.filter.empty? - Net::LDAP::Filter.construct(adaptor.filter % {username: @options[:name_proc].call(request.params["username"])}) + username = Net::LDAP::Filter.escape(@options[:name_proc].call(request.params["username"])) + Net::LDAP::Filter.construct(adaptor.filter % {username: username}) else - Net::LDAP::Filter.eq(adaptor.uid, @options[:name_proc].call(request.params["username"])) + Net::LDAP::Filter.equals(adaptor.uid, @options[:name_proc].call(request.params["username"])) end end @@ -128,6 +139,10 @@ def map_user(mapper, object) protected + def valid_request_method? + request.env["REQUEST_METHOD"] == "POST" + end + def missing_credentials? request.params["username"].nil? || request.params["username"].empty? || request.params["password"].nil? || request.params["password"].empty? end # missing_credentials? diff --git a/omniauth-ldap.gemspec b/omniauth-ldap.gemspec index 583fb1e..aff328f 100644 --- a/omniauth-ldap.gemspec +++ b/omniauth-ldap.gemspec @@ -97,16 +97,11 @@ Gem::Specification.new do |spec| # Listed files are the relative paths from bindir above. spec.executables = [] - spec.add_dependency("net-ldap", "~> 0.16") # ruby >= 2.0 - # nkf/kconv has been part of Ruby since long ago. - # Eventually it became a standard gem, but was changed to a bundled gem in Ruby 3.4. - # In general, kconv and iconv have been superseded since Ruby 1.9 by the built-in - # encoding support provided by String#encode, String#force_encoding, and similar methods. - # spec.add_dependency("nkf") # ruby >= 2.3 - spec.add_dependency("omniauth", ">= 1") # ruby >= 0.0 - spec.add_dependency("pyu-ruby-sasl", "~> 0.0.3.3") # ruby >= 0.0 - spec.add_dependency("rack", ">= 1") # ruby >= 0.0 - spec.add_dependency("rubyntlm", "~> 0.6.2") # ruby >= 1.8.7 + spec.add_dependency("net-ldap", "~> 0.16", "< 1") # ruby >= 2.0 + spec.add_dependency("omniauth", ">= 1", "< 3") # ruby >= 0.0 + spec.add_dependency("pyu-ruby-sasl", ">= 0.0.3.3", "< 0.1") # ruby >= 0.0 + spec.add_dependency("rack", ">= 1", "< 4") # ruby >= 0.0 + spec.add_dependency("rubyntlm", "~> 0.6.2", "< 1") # ruby >= 1.8.7 # Utilities spec.add_dependency("version_gem", "~> 1.1", ">= 1.1.9") # ruby >= 2.2.0 diff --git a/spec/omniauth-ldap/adaptor_spec.rb b/spec/omniauth-ldap/adaptor_spec.rb index 50f70d6..57feb7d 100644 --- a/spec/omniauth-ldap/adaptor_spec.rb +++ b/spec/omniauth-ldap/adaptor_spec.rb @@ -1,22 +1,33 @@ # frozen_string_literal: true -RSpec.describe "OmniAuth::LDAP::Adaptor" do +RSpec.describe OmniAuth::LDAP::Adaptor do describe "initialize" do it "throws exception when must have field is not set" do #[:host, :port, :method, :bind_dn] expect { - OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: "plain"}) + described_class.new({host: "192.168.1.145", method: "plain"}) }.to raise_error(ArgumentError) end it "throws exception when method is not supported" do expect { - OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: "myplain", uid: "uid", port: 389, base: "dc=com"}) - }.to raise_error(OmniAuth::LDAP::Adaptor::ConfigurationError) + described_class.new({host: "192.168.1.145", method: "myplain", uid: "uid", port: 389, base: "dc=com"}) + }.to raise_error(described_class::ConfigurationError) end - it "setups ldap connection with anonymous" do - adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + it "does not throw an error if hosts is set but host and port are not" do + expect { + described_class.new( + hosts: [["192.168.1.145", 389], ["192.168.1.146", 389]], + encryption: "plain", + base: "dc=example,dc=com", + uid: "uid", + ) + }.not_to raise_error + end + + it "sets up ldap connection with anonymous" do + adaptor = described_class.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) expect(adaptor.connection).not_to be_nil expect(adaptor.connection.host).to eq "192.168.1.145" expect(adaptor.connection.port).to eq 389 @@ -24,8 +35,8 @@ expect(adaptor.connection.instance_variable_get(:@auth)).to eq({method: :anonymous, username: nil, password: nil}) end - it "setups ldap connection with simple" do - adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password"}) + it "sets up ldap connection with simple" do + adaptor = described_class.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password"}) expect(adaptor.connection).not_to be_nil expect(adaptor.connection.host).to eq "192.168.1.145" expect(adaptor.connection.port).to eq 389 @@ -33,8 +44,8 @@ expect(adaptor.connection.instance_variable_get(:@auth)).to eq({method: :simple, username: "bind_dn", password: "password"}) end - it "setups ldap connection with sasl-md5" do - adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", try_sasl: true, sasl_mechanisms: ["DIGEST-MD5"], bind_dn: "bind_dn", password: "password"}) + it "sets up ldap connection with sasl-md5" do + adaptor = described_class.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", try_sasl: true, sasl_mechanisms: ["DIGEST-MD5"], bind_dn: "bind_dn", password: "password"}) expect(adaptor.connection).not_to be_nil expect(adaptor.connection.host).to eq "192.168.1.145" expect(adaptor.connection.port).to eq 389 @@ -46,7 +57,7 @@ end it "setups ldap connection with sasl-gss" do - adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", try_sasl: true, sasl_mechanisms: ["GSS-SPNEGO"], bind_dn: "bind_dn", password: "password"}) + adaptor = described_class.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", try_sasl: true, sasl_mechanisms: ["GSS-SPNEGO"], bind_dn: "bind_dn", password: "password"}) expect(adaptor.connection).not_to be_nil expect(adaptor.connection.host).to eq "192.168.1.145" expect(adaptor.connection.port).to eq 389 @@ -57,9 +68,132 @@ expect(adaptor.connection.instance_variable_get(:@auth)[:challenge_response]).not_to be_nil end - it "sets the encryption method correctly" do - adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: "tls", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) - expect(adaptor.connection.instance_variable_get(:@encryption)).to include method: :start_tls + it "sets up a connection with the proper host and port" do + adapter = described_class.new( + host: "192.168.1.145", + encryption: "plain", + base: "dc=example,dc=com", + port: 3890, + uid: "uid", + ) + + expect(adapter.connection.host).to eq("192.168.1.145") + expect(adapter.connection.port).to eq(3890) + expect(adapter.connection.hosts).to be_nil + end + + it "sets up a connection with a enumerable pairs of hosts" do + adapter = described_class.new( + hosts: [["192.168.1.145", 636], ["192.168.1.146", 636]], + encryption: "plain", + base: "dc=example,dc=com", + uid: "uid", + ) + + expect(adapter.connection.host).to eq("127.0.0.1") + expect(adapter.connection.port).to eq(389) + expect(adapter.connection.hosts).to contain_exactly(["192.168.1.145", 636], ["192.168.1.146", 636]) + end + + context "when encryption is plain" do + it "sets encryption to nil" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to be_nil + end + end + + context "when encryption is ssl" do + it "sets the encryption method to simple_tls" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "ssl", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include method: :simple_tls + end + + context "when disable_verify_certificates is not specified" do + it "sets the encryption tls_options to OpenSSL default params" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "ssl", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS + end + end + + context "when disable_verify_certificates is true" do + it "sets the encryption tls_options verify_mode explicitly to verify none" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "ssl", disable_verify_certificates: true, base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: {verify_mode: OpenSSL::SSL::VERIFY_NONE} + end + end + + context "when disable_verify_certificates is false" do + it "sets the encryption tls_options to OpenSSL default params" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "ssl", disable_verify_certificates: false, base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS + end + end + + context "when tls_options are specified" do + it "passes the values along with defaults" do + cert = OpenSSL::X509::Certificate.new + key = OpenSSL::PKey::RSA.new + + adaptor = described_class.new({host: "192.168.1.145", encryption: "ssl", base: "dc=intridea, dc=com", port: 636, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password", tls_options: {ca_file: "/etc/ca.pem", ssl_version: "TLSv1_2", cert: cert, key: key}}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS.merge(ca_file: "/etc/ca.pem", ssl_version: "TLSv1_2", cert: cert, key: key) + end + + it "does not pass nil or blank values" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "ssl", base: "dc=intridea, dc=com", port: 636, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password", tls_options: {ca_file: nil, ssl_version: " "}}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS + end + end + + # DEPRECATED + context "when ca_file is specified" do + it "sets the encryption tls_options ca_file" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "ssl", base: "dc=intridea, dc=com", port: 636, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password", ca_file: "/etc/ca.pem"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS.merge(ca_file: "/etc/ca.pem") + end + end + + # DEPRECATED + context "when ssl_version is specified" do + it "overwrites the encryption tls_options ssl_version" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "ssl", base: "dc=intridea, dc=com", port: 636, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password", ssl_version: "TLSv1_2"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS.merge(ssl_version: "TLSv1_2") + end + end + end + + context "when encryption is tls" do + it "sets the encryption method to start_tls" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "tls", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include method: :start_tls + end + + context "when disable_verify_certificates is not specified" do + it "sets the encryption tls_options to OpenSSL default params" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "tls", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS + end + end + + context "when disable_verify_certificates is true" do + it "sets the encryption tls_options verify_mode explicitly to verify none" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "tls", disable_verify_certificates: true, base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: {verify_mode: OpenSSL::SSL::VERIFY_NONE} + end + end + + context "when disable_verify_certificates is false" do + it "sets the encryption tls_options to OpenSSL default params" do + adaptor = described_class.new({host: "192.168.1.145", encryption: "tls", disable_verify_certificates: false, base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS + end + end + end + + context "when method is set instead of encryption" do + it "sets the encryption method for backwards-compatibility" do + adaptor = described_class.new({host: "192.168.1.145", method: "tls", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName"}) + expect(adaptor.connection.instance_variable_get(:@encryption)).to include method: :start_tls + end end end @@ -68,7 +202,7 @@ let(:rs) { Struct.new(:dn).new("new dn") } it "binds simple" do - adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.126", method: "plain", base: "dc=score, dc=local", port: 389, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password"}) + adaptor = described_class.new({host: "192.168.1.126", encryption: "plain", base: "dc=score, dc=local", port: 389, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password"}) expect(adaptor.connection).to receive(:open).and_yield(adaptor.connection) expect(adaptor.connection).to receive(:search).with(args).and_return([rs]) expect(adaptor.connection).to receive(:bind).with({username: "new dn", password: args[:password], method: :simple}).and_return(true) @@ -76,7 +210,7 @@ end it "binds sasl" do - adaptor = OmniAuth::LDAP::Adaptor.new({host: "192.168.1.145", method: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", try_sasl: true, sasl_mechanisms: ["GSS-SPNEGO"], bind_dn: "bind_dn", password: "password"}) + adaptor = described_class.new({host: "192.168.1.145", encryption: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", try_sasl: true, sasl_mechanisms: ["GSS-SPNEGO"], bind_dn: "bind_dn", password: "password"}) expect(adaptor.connection).to receive(:open).and_yield(adaptor.connection) expect(adaptor.connection).to receive(:search).with(args).and_return([rs]) expect(adaptor.connection).to receive(:bind).and_return(true) diff --git a/spec/omniauth/adaptor_spec.rb b/spec/omniauth/adaptor_spec.rb index 7c1489e..c2b3b71 100644 --- a/spec/omniauth/adaptor_spec.rb +++ b/spec/omniauth/adaptor_spec.rb @@ -13,7 +13,7 @@ subject { described_class.new(valid_config) } it "raises ConfigurationError for unsupported connect method" do - expect { subject.send(:ensure_method, :bogus) }.to raise_error(OmniAuth::LDAP::Adaptor::ConfigurationError) + expect { described_class.new(valid_config.merge(method: :bogus)).send(:translate_method) }.to raise_error(OmniAuth::LDAP::Adaptor::ConfigurationError) end it "returns empty array for no sasl mechanisms" do @@ -21,9 +21,13 @@ end it "maps ssl/tls to Net::LDAP encryption symbols" do - expect(subject.send(:ensure_method, "ssl")).to eq(OmniAuth::LDAP::Adaptor::METHOD[:ssl]) - expect(subject.send(:ensure_method, "tls")).to eq(OmniAuth::LDAP::Adaptor::METHOD[:tls]) - expect(subject.send(:ensure_method, "plain")).to eq(OmniAuth::LDAP::Adaptor::METHOD[:plain]) + ssl_adaptor = described_class.new(valid_config.merge(method: "ssl")) + tls_adaptor = described_class.new(valid_config.merge(method: "tls")) + plain_adaptor = described_class.new(valid_config.merge(method: "plain")) + + expect(ssl_adaptor.send(:translate_method)).to eq(OmniAuth::LDAP::Adaptor::ENCRYPTION_METHOD[:ssl]) + expect(tls_adaptor.send(:translate_method)).to eq(OmniAuth::LDAP::Adaptor::ENCRYPTION_METHOD[:tls]) + expect(plain_adaptor.send(:translate_method)).to eq(OmniAuth::LDAP::Adaptor::ENCRYPTION_METHOD[:plain]) end it "initializes with try_sasl and sets bind_method to :sasl" do diff --git a/spec/omniauth/strategies/ldap_spec.rb b/spec/omniauth/strategies/ldap_spec.rb index 3e235c7..22e7ed4 100644 --- a/spec/omniauth/strategies/ldap_spec.rb +++ b/spec/omniauth/strategies/ldap_spec.rb @@ -5,6 +5,7 @@ # :host => '10.101.10.1', # :port => 389, # :method => :plain, + # :verify_certificates => true, # :base => 'dc=intridea, dc=com', # :uid => 'sAMAccountName', # :name_proc => Proc.new {|name| name.gsub(/@.*$/,'')} @@ -45,7 +46,20 @@ class MyLdapProvider < OmniAuth::Strategies::LDAP; end end describe "/auth/ldap" do - before { post("/auth/ldap", {}) } + let!(:csrf_token) { SecureRandom.base64(32) } + let(:post_env) { make_env("/auth/ldap", "rack.session" => {csrf: csrf_token}, "rack.input" => StringIO.new("authenticity_token=#{escaped_token}")) } + let(:escaped_token) { URI.encode_www_form_component(csrf_token, Encoding::UTF_8) } + + before { post "/auth/ldap", nil, post_env } + + def make_env(path = "/auth/ldap", props = {}) + { + "REQUEST_METHOD" => "POST", + "PATH_INFO" => path, + "rack.session" => {}, + "rack.input" => StringIO.new("test=true"), + }.merge(props) + end it "displays a form" do expect(last_response.status).to eq 200 @@ -78,6 +92,36 @@ class MyLdapProvider < OmniAuth::Strategies::LDAP; end allow(@adaptor).to receive(:bind_as).and_return(false) end + it "fails with missing_credentials" do + post("/auth/ldap/callback", {}) + expect(last_response).to be_redirect + expect(last_response.headers["Location"]).to match(%r{missing_credentials}) + end + + it "redirects to error page" do + post("/auth/ldap/callback", {username: "ping", password: "password"}) + + expect(last_response).to be_redirect + expect(last_response.headers["Location"]).to match("invalid_credentials") + expect(last_request.env["omniauth.error"].message).to eq("Invalid credentials for ping") + end + + it "redirects to error page when there is exception" do + allow(@adaptor).to receive(:bind_as).and_raise(StandardError.new("connection_error")) + post("/auth/ldap/callback", {username: "ping", password: "password"}) + expect(last_response).to be_redirect + expect(last_response.headers["Location"]).to match(%r{ldap_error}) + end + + context "when wrong request method" do + it "redirects to error page" do + get("/auth/ldap/callback", {username: "ping", password: "password"}) + + expect(last_response).to be_redirect + expect(last_response.headers["Location"]).to match("invalid_request_method") + end + end + context "when username is not preset" do it "redirects to error page" do post("/auth/ldap/callback", {}) @@ -123,7 +167,8 @@ class MyLdapProvider < OmniAuth::Strategies::LDAP; end post("/auth/ldap/callback", {username: "ping", password: "password"}) expect(last_response).to be_redirect - expect(last_response.headers["Location"]).to match %r{invalid_credentials} + expect(last_response.headers["Location"]).to match("invalid_credentials") + expect(last_request.env["omniauth.error"].message).to eq("Invalid credentials for ping") end context "and filter is set" do @@ -133,14 +178,15 @@ class MyLdapProvider < OmniAuth::Strategies::LDAP; end post("/auth/ldap/callback", {username: "ping", password: "password"}) expect(last_response).to be_redirect - expect(last_response.headers["Location"]).to match %r{invalid_credentials} + expect(last_response.headers["Location"]).to match("invalid_credentials") + expect(last_request.env["omniauth.error"].message).to eq("Invalid credentials for ping") end end end context "and communication with LDAP server caused an exception" do before do - allow(@adaptor).to receive(:bind_as).and_throw(Exception.new("connection_error")) + allow(@adaptor).to receive(:bind_as).and_raise(StandardError.new("connection_error")) end it "redirects to error page" do @@ -198,7 +244,52 @@ class MyLdapProvider < OmniAuth::Strategies::LDAP; end it "maps user info to Auth Hash" do post("/auth/ldap/callback", {username: "ping", password: "password"}) + expect(auth_hash.uid).to eq "cn=ping, dc=intridea, dc=com" + + info = auth_hash.info + + expect(info.email).to eq "ping@intridea.com" + expect(info.first_name).to eq "Ping" + expect(info.last_name).to eq "Yu" + expect(info.phone).to eq "555-555-5555" + expect(info.mobile).to eq "444-444-4444" + expect(info.nickname).to eq "ping" + expect(info.title).to eq "dev" + expect(info.location).to eq "k street, Washington, DC, U.S.A 20001" + expect(info.url).to eq "www.intridea.com" + expect(info.image).to eq "http://www.intridea.com/ping.jpg" + expect(info.description).to eq "omniauth-ldap" + end + end + + context "when alternate fields" do + let(:auth_hash) { last_request.env["omniauth.auth"] } + + before do + allow(@adaptor).to receive(:filter) + allow(@adaptor).to receive(:bind_as).and_return(Net::LDAP::Entry.from_single_ldif_string( + %{dn: cn=ping, dc=intridea, dc=com +userprincipalname: ping@intridea.com +givenname: Ping +sn: Yu +telephonenumber: 555-555-5555 +mobile: 444-444-4444 +uid: ping +title: dev +address: k street +l: Washington +st: DC +co: U.S.A +postofficebox: 20001 +wwwhomepage: www.intridea.com +jpegphoto: http://www.intridea.com/ping.jpg +description: omniauth-ldap +}, + )) + end + it "maps user info to Auth Hash" do + post("/auth/ldap/callback", {username: "ping", password: "password"}) expect(auth_hash.uid).to eq "cn=ping, dc=intridea, dc=com" info = auth_hash.info diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b4a54d8..714d494 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -23,7 +23,6 @@ # External library dependencies require "omniauth" -require "omniauth/version" # RSpec Configs require "config/debug"