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.

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

Portus crashes when guessing LDAP email #1832

Closed
alexanderteves opened this Issue May 24, 2018 · 4 comments

Comments

Projects
2 participants
@alexanderteves

alexanderteves commented May 24, 2018

Description

Portus crashes when guessing LDAP email while working fine without it.

Steps to reproduce

I set up Portus to use LDAP which works fine without guessing the email. But when enabling guessing with the actual attribute, Portus crashes with:

NoMethodError (undefined method `size' for nil:NilClass):
  lib/portus/ldap.rb:235:in `guess_email'
  lib/portus/ldap.rb:213:in `find_or_create_user!'
  lib/portus/ldap.rb:195:in `portus_login!'
  lib/portus/ldap.rb:43:in `authenticate!'
  app/controllers/application_controller.rb:56:in `force_update_profile!'
  app/middleware/catch_json_parse_errors.rb:11:in `call'

At least I would expect it to fall back to no email setup and redirecting the user to the profile page to enter it manually as stated in the docs.

Deployment information

Deployment method:

Bare metal with official Docker image v2.3

Configuration:

ldap:
  enabled: true

  hostname: "ldap.example.com"
  port: 389
  method: "start_tls"

  encryption:
    method: "plain"
  
base: "dc=example,dc=com"

  filter: "(&(objectClass=person)(|(employeeType=temporary)(employeeType=permanent)(employeeType=technical)(employeeType=external)))"

  uid: "uid"

  authentication:
    enabled: true
    bind_dn: "cn=readonly,ou=admin,dc=example,dc=com"
    password: "secret123"

  guess_email:
    enabled: true
    attr: "mail"

Portus version: v2.3

@mssola

This comment has been minimized.

Contributor

mssola commented May 24, 2018

There are two things to note here:

  1. This is because a search fails, and it will return nil apparently if it failed as specified here. So, it's definitely a bug, because we don't protected ourselves from that when guessing the email...
  2. Notice that your configuration is flawed: the base attribute should be at the same indentation level as hostname or port. That will definitely cause some problems (and possibly it will lead to this bug 😉).

Thanks for the report ! 👍

@mssola mssola self-assigned this May 24, 2018

@mssola mssola added this to the Release 2.4 milestone May 24, 2018

@mssola mssola added the bug label May 24, 2018

@mssola mssola added this to To do in 2.4 May 24, 2018

@alexanderteves

This comment has been minimized.

alexanderteves commented May 24, 2018

@mssola Thanks for the heads up. Concerning 2): My mistake while pasting it here. I checked with our production *.yml and the indention is correct there, still fails. Sorry for that!

@alexanderteves

This comment has been minimized.

alexanderteves commented May 25, 2018

I debugged the @ldap.search in L234 with @ldap.get_operation_result and the message is unauthenticated bind (DN with no password) disallowed. True, I don't want that. But is that not what the credentials from authentcation: should be used for?

@mssola mssola moved this from To do to In progress in 2.4 May 28, 2018

@mssola mssola closed this in 291b049 May 28, 2018

2.4 automation moved this from In progress to Done May 28, 2018

mssola added a commit that referenced this issue May 28, 2018

ldap: don't crash on search when guessing an email
The `search` method from Net::LDAP might return nil. In this case simply
return early as we do on other cases.

See #1832

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>
@mssola

This comment has been minimized.

Contributor

mssola commented May 28, 2018

I've also adapted the fix into 2.3: 93df51c. It should be available on the Docker image rather shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment