Skip to content
Snippets Groups Projects

Resolve "LDAP connector"

Merged Resolve "LDAP connector"
380-ldap-connector into master
1 unresolved thread
Merged Piotr Gawron requested to merge 380-ldap-connector into master
1 unresolved thread

This merge request allows to configure smasch in a way that authentication is done via LDAP.

However, due to the fact that data stored in smasch is very sensitive, LDAP authentication is supported only for users that already exist in smasch (they will not be created automatically). Moreover worker flag ldap_user must be set to True.

To configure ldap there are two parameters that must be added to local_settings.py:

AUTH_LDAP_SERVER_URI = "ldap://lcsb-cdc-lums-01.uni.lu"
AUTH_LDAP_BASE_DN = "cn=users,cn=accounts,dc=uni,dc=lu"

By default LDAP connection is disabled.

Closes #380 (closed)

Edited by Piotr Gawron

Merge request reports

Pipeline #47841 passed

Stage: test

    Pipeline passed for 5456608b on 380-ldap-connector

    Test coverage 88.18% (0.03%) from 5 jobs
    Approved by
    Carlos Vega

    Merged by Piotr GawronPiotr Gawron 3 years ago (Oct 5, 2021 8:24am UTC)

    Merge details

    Pipeline #47922 passed

    Stage: test

      Pipeline passed for 6294953c on master

      Test coverage 88.18% (0.03%) from 5 jobs

      Activity

      Filter activity
      • Approvals
      • Assignees & reviewers
      • Comments (from bots)
      • Comments (from users)
      • Commits & branches
      • Edits
      • Labels
      • Lock status
      • Mentions
      • Merge request status
      • Tracking
      smash/web/auth/CustomModelBackend.py 0 → 100644
      1 import logging
      2
      3 from django.contrib.auth.backends import ModelBackend
      4
      5 logger = logging.getLogger(__name__)
      6
      7
      8 class CustomModelBackend(ModelBackend):
      9
      10 def authenticate(self, request, username=None, password=None, **kwargs):
      11 # FIXME: If we want to prevent login of users without Worker this should work
    • Carlos Vega
      Carlos Vega @carlosvega ·
      Owner

      My only concern is whether this works as intended. Given LDAP is enabled, if a user is disabled in LDAP, would still be able to login? What if the user is disabled locally but not in LDAP?

    • Piotr Gawron
      Piotr Gawron @piotr.gawron ·
      Author Owner

      Given LDAP is enabled, if a user is disabled in LDAP, would still be able to login?

      I would say he should not. And as far as I know he wouldn't be able to login. But it would be nice to have some automatic tests for that.

      What if the user is disabled locally but not in LDAP?

      The code below performs authentication only if localuser is: active, has a worker and ldap_user=True. Otherwise LDAP authentication is not called (return None)

         def authenticate(self, request, username=None, password=None, **kwargs):
        count = Worker.objects.filter(user__username=username, user__is_active=True, ldap_user=True).count()
        if count == 1:
            logger.debug("Trying LDAP auth with: " + username)
            return super().authenticate(request, username, password, **kwargs)
        else:
            return None

      Maybe you can try to set it up locally and play a bit using our lums LDAP?

    • Carlos Vega
      Carlos Vega @carlosvega ·
      Owner

      I don't have much time at the moment but I could try. I haven't set up LDAP in a while, any instructions for it? I remember it was a bit painful system to work with.

    • Piotr Gawron
      Piotr Gawron @piotr.gawron ·
      Author Owner

      try this:

      To configure ldap there are two parameters that must be added to local_settings.py:

      AUTH_LDAP_SERVER_URI = "ldap://lcsb-cdc-lums-01.uni.lu"
AUTH_LDAP_BASE_DN = "cn=users,cn=accounts,dc=uni,dc=lu"

      and you should be connected to lums ldap (you might want to change ldap:// to ldaps:// (ssl support), I have not tested it yet

    • Piotr Gawron added 1 commit

      added 1 commit

      Compare with previous version

    • Piotr Gawron added 2 commits

      added 2 commits

      Compare with previous version

    • Piotr Gawron added 1 commit

      added 1 commit

      Compare with previous version

    • Piotr Gawron added 1 commit

      added 1 commit

      Compare with previous version

    • Piotr Gawron added 24 commits

      added 24 commits

      Compare with previous version

    • Carlos Vega
      Carlos Vega @carlosvega ·
      Owner

      So, apart from the speed, I think all is fine.

    • Carlos Vega approved this merge request

      approved this merge request

    • Piotr Gawron mentioned in commit 6294953c

      mentioned in commit 6294953c

    • Piotr Gawron merged

      merged

      • Please register or sign in to reply