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.

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

Fix LDAP authentication exception #3073

Merged
merged 2 commits into from
Dec 15, 2020
Merged

Conversation

jimis
Copy link
Contributor

@jimis jimis commented Dec 3, 2020

plus minor logging improvements.

gyorb
gyorb previously requested changes Dec 7, 2020
web/server/codechecker_server/auth/cc_ldap.py Show resolved Hide resolved
web/server/codechecker_server/auth/cc_ldap.py Outdated Show resolved Hide resolved
@gyorb gyorb added server 🖥️ web 🌍 Related to the web app labels Dec 7, 2020
jimis added 2 commits December 7, 2020 20:19
When groupPattern is null in server_config.json, the following
exception was being logged when a user tried to authenticate:

    File ".../codechecker_server/auth/cc_ldap.py", line 390, in get_groups
      group_pattern = group_pattern.replace('$USERDN$', user_dn)
  AttributeError: 'NoneType' object has no attribute 'replace'

groupPattern is also null in one of the examples in authentication.md.
In this case, the get() with default value that was used:

  ldap_config.get('groupPattern', '')

actually returned None, since the key actually existed in the dictionary
with value None.
Copy link
Contributor

@csordasmarton csordasmarton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@csordasmarton csordasmarton added this to the release 6.16.0 milestone Dec 15, 2020
@csordasmarton csordasmarton dismissed gyorb’s stale review December 15, 2020 09:10

Your comments are fixed.

@csordasmarton csordasmarton merged commit a0f7414 into Ericsson:master Dec 15, 2020
@csordasmarton
Copy link
Contributor

@jimis Thank you for this patch and for your time 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server 🖥️ web 🌍 Related to the web app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants