search

jonmbake / discourse-ldap-auth

Discourse plugin to enable LDAP/Active Directory authentication.
MIT License
70 stars 53 forks source link

LDAP filter not escaped properly #34

Open bviktor opened 6 years ago

bviktor commented 6 years ago

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 commented 6 years ago

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

bviktor commented 6 years ago

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

jonmbake commented 6 years ago

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 commented 6 years ago

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.

dnsmichi commented 5 years ago

Requirement

Users can be member of

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 commented 5 years ago

Seeing the exact same behavior. Any help is greatly appreciated

dnsmichi commented 5 years ago

Did you try the workaround filter mentioned in https://github.com/jonmbake/discourse-ldap-auth/issues/34#issuecomment-442885351 ?

akhalidy-IDM commented 5 years ago

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.