Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LDAP filter not escaped properly #34

Open
bviktor opened this issue Sep 4, 2018 · 8 comments
Open

LDAP filter not escaped properly #34

bviktor opened this issue Sep 4, 2018 · 8 comments

Comments

@bviktor
Copy link

bviktor commented Sep 4, 2018

Take the following DN as an example:

memberOf=CN=Discourse Users,OU=Groups,DC=ad,DC=foobar,DC=com

This results in the following error on the UI:

Sorry, there was an error authorizing your account. Perhaps you did not approve authorization?

And in the error logs:

(ldap) Authentication failure! invalid_credentials encountered.

/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/logster-1.2.11/lib/logster/logger.rb:94:in `add_with_opts'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/logster-1.2.11/lib/logster/logger.rb:51:in `add'
/usr/local/lib/ruby/2.5.0/logger.rb:545:in `error'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:162:in `log'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:483:in `fail!'
/var/www/discourse/plugins/discourse-ldap-auth/gems/2.5.1/gems/omniauth-ldap-1.0.5/lib/omniauth/strategies/ldap.rb:43:in `callback_phase'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:236:in `callback_call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:188:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/builder.rb:63:in `call'
/var/www/discourse/lib/middleware/omniauth_bypass_middleware.rb:22:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/tempfile_reaper.rb:15:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/conditional_get.rb:38:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/head.rb:12:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/http/content_security_policy.rb:18:in `call'
/var/www/discourse/lib/middleware/anonymous_cache.rb:214:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/session/abstract/id.rb:232:in `context'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/session/abstract/id.rb:226:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/cookies.rb:670:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/activesupport-5.2.0/lib/active_support/callbacks.rb:98:in `run_callbacks'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/callbacks.rb:26:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/debug_exceptions.rb:61:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/logster-1.2.11/lib/logster/middleware/reporter.rb:31:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/rack/logger.rb:38:in `call_app'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/rack/logger.rb:28:in `call'
/var/www/discourse/config/initializers/100-quiet_logger.rb:16:in `call'
/var/www/discourse/config/initializers/100-silence_logger.rb:29:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/request_id.rb:27:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/method_override.rb:22:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/executor.rb:14:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/sendfile.rb:111:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-mini-profiler-1.0.0/lib/mini_profiler/profiler.rb:174:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/message_bus-2.1.5/lib/message_bus/rack/middleware.rb:63:in `call'
/var/www/discourse/lib/middleware/request_tracker.rb:180:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/engine.rb:524:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/railtie.rb:190:in `public_send'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/railtie.rb:190:in `method_missing'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/urlmap.rb:68:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/urlmap.rb:53:in `each'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.5/lib/rack/urlmap.rb:53:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/lib/unicorn/http_server.rb:606:in `process_client'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/lib/unicorn/http_server.rb:701:in `worker_loop'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/lib/unicorn/http_server.rb:549:in `spawn_missing_workers'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/lib/unicorn/http_server.rb:142:in `start'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/bin/unicorn:126:in `<top (required)>'
/var/www/discourse/vendor/bundle/ruby/2.5.0/bin/unicorn:23:in `load'
/var/www/discourse/vendor/bundle/ruby/2.5.0/bin/unicorn:23:in `<main>'

Workaround: put \20 in place of spaces.

memberOf=CN=Discourse\20Users,OU=Groups,DC=ad,DC=foobar,DC=com

This might be required for other fields as well, haven't tested it extensively.
@bviktor
Copy link
Author

bviktor commented Sep 4, 2018

Actually, nevermind, I have no idea what's going on, some users can sign in, some can't.

@bviktor
Copy link
Author

bviktor commented Oct 19, 2018

Gee, that's one way to "solve" issues I guess.

@jonmbake
Copy link
Owner

Actually, nevermind, I have no idea what's going on, some users can sign in, some can't.

I took this as it was an issue on your end. Is this still an issue for you? I can re-open it.

@bviktor
Copy link
Author

bviktor commented Oct 19, 2018

It still is an issue. We ended up removing the filter coz it's useless at this point. I'm not a big fan of it since it lets anyone sign in.

@jonmbake jonmbake reopened this Oct 19, 2018
@dnsmichi
Copy link

dnsmichi commented Nov 29, 2018 •
edited
Loading

Requirement

Users can be member of

  • all-access (our infra admin team)
  • discourse (additional group for selected users)

The LDAP filter used in other applications looks like this:

(&(|(objectclass=inetOrgPerson))(|(memberof=cn=all-access,ou=groups,dc=domain,dc=com)(memberof=cn=discourse,ou=groups,dc=domain,dc=com)))

Problem

I'm seeing the same behaviour as described above.

/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/logster-1.3.1/lib/logster/logger.rb:101:in `add_with_opts'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/logster-1.3.1/lib/logster/logger.rb:52:in `add'
/usr/local/lib/ruby/2.5.0/logger.rb:545:in `error'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:162:in `log'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:483:in `fail!'
/var/www/discourse/plugins/discourse-ldap-auth/gems/2.5.2/gems/omniauth-ldap-1.0.5/lib/omniauth/strategies/ldap.rb:43:in `callback_phase'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:236:in `callback_call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:188:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:190:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/strategy.rb:168:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/omniauth-1.8.1/lib/omniauth/builder.rb:63:in `call'
/var/www/discourse/lib/middleware/omniauth_bypass_middleware.rb:22:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/tempfile_reaper.rb:15:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/conditional_get.rb:38:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/head.rb:12:in `call'
/var/www/discourse/lib/content_security_policy.rb:14:in `call'
/var/www/discourse/lib/middleware/anonymous_cache.rb:216:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/session/abstract/id.rb:232:in `context'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/session/abstract/id.rb:226:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/cookies.rb:670:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/activesupport-5.2.0/lib/active_support/callbacks.rb:98:in `run_callbacks'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/callbacks.rb:26:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/debug_exceptions.rb:61:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/logster-1.3.1/lib/logster/middleware/reporter.rb:31:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/rack/logger.rb:38:in `call_app'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/rack/logger.rb:28:in `call'
/var/www/discourse/config/initializers/100-quiet_logger.rb:16:in `call'
/var/www/discourse/config/initializers/100-silence_logger.rb:29:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/request_id.rb:27:in `call'
/var/www/discourse/lib/middleware/enforce_hostname.rb:17:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/method_override.rb:22:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.0/lib/action_dispatch/middleware/executor.rb:14:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/sendfile.rb:111:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-mini-profiler-1.0.0/lib/mini_profiler/profiler.rb:174:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/message_bus-2.1.6/lib/message_bus/rack/middleware.rb:63:in `call'
/var/www/discourse/lib/middleware/request_tracker.rb:180:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/engine.rb:524:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/railtie.rb:190:in `public_send'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/railties-5.2.0/lib/rails/railtie.rb:190:in `method_missing'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/urlmap.rb:68:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/urlmap.rb:53:in `each'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/rack-2.0.6/lib/rack/urlmap.rb:53:in `call'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/lib/unicorn/http_server.rb:606:in `process_client'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/lib/unicorn/http_server.rb:701:in `worker_loop'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/lib/unicorn/http_server.rb:549:in `spawn_missing_workers'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/lib/unicorn/http_server.rb:142:in `start'
/var/www/discourse/vendor/bundle/ruby/2.5.0/gems/unicorn-5.4.0/bin/unicorn:126:in `<top (required)>'
/var/www/discourse/vendor/bundle/ruby/2.5.0/bin/unicorn:23:in `load'
/var/www/discourse/vendor/bundle/ruby/2.5.0/bin/unicorn:23:in `<main>'

Analysis

I don't know anything about omniauth nor how exactly it works within Ruby and the LDAP strategy. Fortunately everything is open source, so I did a little reading there. I fear to patch the Discourse container for better debugging, so I just tried to gather an idea how this LDAP filter is constructed and passed to omniauth.

Plugin to omniauth

plugin.rb maps the field from Discourse settings 1:1 to omniauth-ldap.

filter: SiteSetting.ldap_filter

Therefore I can talk directly to omniauth via admin settings, that's good to know.

Documentation hint

The omniauth-ldap documentation says that you can use the filter parameter to also directly filter for the username attribute gathered from name_proc.

:filter is the LDAP filter used to search the user entry. It can be used in place of :uid for more flexibility. %{username} will be replaced by the user name processed by :name_proc.

:name_proc allows you to match the user name entered with the format of the :uid attributes. For example, value of 'sAMAccountName' in AD contains only the windows user name. If your user prefers using email to login, a name_proc as above will trim the email string down to just the windows login name. In summary, use :name_proc to fill the gap between the submitted username and LDAP uid attribute value.

Solution

A working filter

In addition to the main check, e.g. the objectclass, combine it with uid (or sAmAccountName) and pass the template variable %{username}.

(&(&(uid=%{username})(objectclass=inetOrgPerson))(|(memberof=cn=all-access,ou=groups,dc=domain,dc=com)(memberof=cn=discourse,ou=groups,dc=domain,dc=com)))

Patch

Either the documentation needs a hint for a "special" LDAP filter format, or the plugin takes care of constructing the filter itself. Maybe it needs to parse the settings then, I'm not sure about this. Maybe it can just prepend the string with

filter: "(&(" + SiteSetting.ldap_uid + "=%{username})(" + SiteSetting.ldap_filter + ")"

I did not test the above code though.

@akhalidy-IDM
Copy link

Seeing the exact same behavior. Any help is greatly appreciated

@dnsmichi
Copy link

dnsmichi commented Jan 7, 2019

Did you try the workaround filter mentioned in #34 (comment) ?

@akhalidy-IDM
Copy link

I actually solved my problem by comparing the working/non-working user attributes and since I am using email to logon, some did not have the attribute "mail" setup. Once I corrected that, all the users that weren't able to logon are now successfully logging in.

@Victor-Morel Victor-Morel mentioned this issue May 11, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dnsmichi @jonmbake @bviktor @akhalidy-IDM