From bfd77df19e695dfdc031eedcd192e83d997b8674 Mon Sep 17 00:00:00 2001 From: "Brian C. Arnold" Date: Mon, 24 Oct 2022 05:00:29 -0400 Subject: [PATCH 1/9] Added ability to specify LDAP attribute mapping. Specifically made sure that LDAP auth works the same if the new environment variables aren't set, in order to maintain behavior for users who are already using LDAP if they don't set the new envvars. --- tubearchivist/config/settings.py | 47 ++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/tubearchivist/config/settings.py b/tubearchivist/config/settings.py index 744c8e052..563de7ca7 100644 --- a/tubearchivist/config/settings.py +++ b/tubearchivist/config/settings.py @@ -103,20 +103,55 @@ global AUTH_LDAP_BIND_PASSWORD AUTH_LDAP_BIND_PASSWORD = environ.get("TA_LDAP_BIND_PASSWORD") + + # Bind options + + # Since these are new environment variables, taking the opporunity to use more accurate + # env names. + # Given Names are *_technically_* different from Personal names, as people who change their names + # have different given names and personal names, and they go by personal names. + # Additionally, "LastName" is actually incorrect for many cultures, such as Korea, where the + # family name comes first, and the personal name comes last. + + # But we all know people are going to try to guess at these, so still want to include + # names that people will guess, hence using first/last as well. + + global AUTH_LDAP_BIND_ATTR_USERNAME + AUTH_LDAP_BIND_ATTR_USERNAME = environ.get("TA_LDAP_BIND_ATTR_USERNAME") or environ.get("TA_LDAP_BIND_ATTR_UID") or "uid", + + global AUTH_LDAP_BIND_ATTR_PERSONALNAME + AUTH_LDAP_BIND_ATTR_PERSONALNAME = environ.get("TA_LDAP_BIND_ATTR_PERSONALNAME") or environ.get("TA_LDAP_BIND_ATTR_FIRSTNAME") or "givenName", + + global AUTH_LDAP_BIND_ATTR_SURNAME + AUTH_LDAP_BIND_ATTR_SURNAME = environ.get("TA_LDAP_BIND_ATTR_SURNAME") or environ.get("TA_LDAP_BIND_ATTR_LASTNAME") or "sn", + + global AUTH_LDAP_BIND_ATTR_EMAIL + AUTH_LDAP_BIND_ATTR_EMAIL = environ.get("TA_LDAP_BIND_ATTR_EMAIL") or "mail", + + + + global AUTH_LDAP_USER_BASE + AUTH_LDAP_USER_BASE = environ.get("TA_LDAP_USER_BASE") + + global AUTH_LDAP_USER_FILTER + AUTH_LDAP_USER_FILTER = environ.get("TA_LDAP_USER_FILTER") + global AUTH_LDAP_USER_SEARCH # pylint: disable=no-member AUTH_LDAP_USER_SEARCH = LDAPSearch( - environ.get("TA_LDAP_USER_BASE"), + AUTH_LDAP_USER_BASE, ldap.SCOPE_SUBTREE, - "(&(uid=%(user)s)" + environ.get("TA_LDAP_USER_FILTER") + ")", + "(&(%(AUTH_LDAP_BIND_ATTR_USERNAME)s=%(user)s)" + AUTH_LDAP_USER_FILTER + ")", ) + + global AUTH_LDAP_USER_ATTR_MAP AUTH_LDAP_USER_ATTR_MAP = { - "username": "uid", - "first_name": "givenName", - "last_name": "sn", - "email": "mail", + "username": AUTH_LDAP_BIND_ATTR_USERNAME, + "first_name": AUTH_LDAP_BIND_ATTR_PERSONALNAME, + "last_name": AUTH_LDAP_BIND_ATTR_SURNAME, + "email": AUTH_LDAP_BIND_ATTR_EMAIL, } if bool(environ.get("TA_LDAP_DISABLE_CERT_CHECK")): From 88e57de09158f3db75c31bd8c53116596022e2f2 Mon Sep 17 00:00:00 2001 From: "Brian C. Arnold" Date: Mon, 24 Oct 2022 05:11:14 -0400 Subject: [PATCH 2/9] Updated env var name to match the name of the parent global. --- tubearchivist/config/settings.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tubearchivist/config/settings.py b/tubearchivist/config/settings.py index 563de7ca7..9d41e5aff 100644 --- a/tubearchivist/config/settings.py +++ b/tubearchivist/config/settings.py @@ -116,17 +116,17 @@ # But we all know people are going to try to guess at these, so still want to include # names that people will guess, hence using first/last as well. - global AUTH_LDAP_BIND_ATTR_USERNAME - AUTH_LDAP_BIND_ATTR_USERNAME = environ.get("TA_LDAP_BIND_ATTR_USERNAME") or environ.get("TA_LDAP_BIND_ATTR_UID") or "uid", + global AUTH_LDAP_USER_ATTR_MAP_USERNAME + AUTH_LDAP_USER_ATTR_MAP_USERNAME = environ.get("TA_LDAP_USER_ATTR_MAP_USERNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_UID") or "uid", - global AUTH_LDAP_BIND_ATTR_PERSONALNAME - AUTH_LDAP_BIND_ATTR_PERSONALNAME = environ.get("TA_LDAP_BIND_ATTR_PERSONALNAME") or environ.get("TA_LDAP_BIND_ATTR_FIRSTNAME") or "givenName", + global AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME + AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME = environ.get("TA_LDAP_USER_ATTR_MAP_PERSONALNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_FIRSTNAME") or "givenName", - global AUTH_LDAP_BIND_ATTR_SURNAME - AUTH_LDAP_BIND_ATTR_SURNAME = environ.get("TA_LDAP_BIND_ATTR_SURNAME") or environ.get("TA_LDAP_BIND_ATTR_LASTNAME") or "sn", + global AUTH_LDAP_USER_ATTR_MAP_SURNAME + AUTH_LDAP_USER_ATTR_MAP_SURNAME = environ.get("TA_LDAP_USER_ATTR_MAP_SURNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_LASTNAME") or "sn", - global AUTH_LDAP_BIND_ATTR_EMAIL - AUTH_LDAP_BIND_ATTR_EMAIL = environ.get("TA_LDAP_BIND_ATTR_EMAIL") or "mail", + global AUTH_LDAP_USER_ATTR_MAP_EMAIL + AUTH_LDAP_USER_ATTR_MAP_EMAIL = environ.get("TA_LDAP_USER_ATTR_MAP_EMAIL") or environ.get("TA_LDAP_USER_ATTR_MAP_MAIL") or "mail", @@ -141,17 +141,17 @@ AUTH_LDAP_USER_SEARCH = LDAPSearch( AUTH_LDAP_USER_BASE, ldap.SCOPE_SUBTREE, - "(&(%(AUTH_LDAP_BIND_ATTR_USERNAME)s=%(user)s)" + AUTH_LDAP_USER_FILTER + ")", + "(&(%(AUTH_LDAP_USER_ATTR_MAP_USERNAME)s=%(user)s)" + AUTH_LDAP_USER_FILTER + ")", ) global AUTH_LDAP_USER_ATTR_MAP AUTH_LDAP_USER_ATTR_MAP = { - "username": AUTH_LDAP_BIND_ATTR_USERNAME, - "first_name": AUTH_LDAP_BIND_ATTR_PERSONALNAME, - "last_name": AUTH_LDAP_BIND_ATTR_SURNAME, - "email": AUTH_LDAP_BIND_ATTR_EMAIL, + "username": AUTH_LDAP_USER_ATTR_MAP_USERNAME, + "first_name": AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME, + "last_name": AUTH_LDAP_USER_ATTR_MAP_SURNAME, + "email": AUTH_LDAP_USER_ATTR_MAP_EMAIL, } if bool(environ.get("TA_LDAP_DISABLE_CERT_CHECK")): From 524e2cde4ed58ffb1848ffc1f943b8e666d8ce09 Mon Sep 17 00:00:00 2001 From: "Brian C. Arnold" Date: Mon, 24 Oct 2022 05:11:42 -0400 Subject: [PATCH 3/9] Updated README.md to include information on new user attribute mapping environment variables. --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6279af2c3..f012dc293 100644 --- a/README.md +++ b/README.md @@ -125,8 +125,12 @@ You can configure LDAP with the following environment variables: - `TA_LDAP_DISABLE_CERT_CHECK` (ex: `true`) Set to anything besides empty string to disable certificate checking when connecting over LDAPS. - `TA_LDAP_BIND_DN` (ex: `uid=search-user,ou=users,dc=your-server`) DN of the user that is able to perform searches on your LDAP account. - `TA_LDAP_BIND_PASSWORD` (ex: `yoursecretpassword`) Password for the search user. + - `TA_LDAP_USER_ATTR_MAP_USERNAME` (default: `uid`) Bind attribute used to map LDAP user's username + - `TA_LDAP_USER_ATTR_MAP_PERSONALNAME` (default: `givenName`) Bind attribute used to match LDAP user's First Name/Personal Name. + - `TA_LDAP_USER_ATTR_MAP_SURNAME` (default: `sn`) Bind attribute used to match LDAP user's Last Name/Surname. + - `TA_LDAP_USER_ATTR_MAP_EMAIL` (default: `mail`) Bind attribute used to match LDAP user's EMail address - `TA_LDAP_USER_BASE` (ex: `ou=users,dc=your-server`) Search base for user filter. - - `TA_LDAP_USER_FILTER` (ex: `(objectClass=user)`) Filter for valid users. Login usernames are automatically matched using `uid` and does not need to be specified in this filter. + - `TA_LDAP_USER_FILTER` (ex: `(objectClass=user)`) Filter for valid users. Login usernames are matched using the attribute specified in `TA_LDAP_USER_ATTR_MAP_USERNAME` and should not be specified in this filter. When LDAP authentication is enabled, django passwords (e.g. the password defined in TA_PASSWORD), will not allow you to login, only the LDAP server is used. From 35db28e676df26428d0f9fd4ef9153ac676c3126 Mon Sep 17 00:00:00 2001 From: "Brian C. Arnold" Date: Mon, 24 Oct 2022 05:14:01 -0400 Subject: [PATCH 4/9] Added additional environment var options, and updated comment explaining why there are multiple. --- tubearchivist/config/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tubearchivist/config/settings.py b/tubearchivist/config/settings.py index 9d41e5aff..83a961b68 100644 --- a/tubearchivist/config/settings.py +++ b/tubearchivist/config/settings.py @@ -104,7 +104,7 @@ AUTH_LDAP_BIND_PASSWORD = environ.get("TA_LDAP_BIND_PASSWORD") - # Bind options + # Attribute mapping options # Since these are new environment variables, taking the opporunity to use more accurate # env names. From 5272d2e8e176b3937aadf06259e092a1c640e642 Mon Sep 17 00:00:00 2001 From: "Brian C. Arnold" Date: Mon, 24 Oct 2022 05:50:30 -0400 Subject: [PATCH 5/9] I'm not a python programmer, so these were stupid mistakes. Works now. --- tubearchivist/config/settings.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tubearchivist/config/settings.py b/tubearchivist/config/settings.py index 83a961b68..4298b9744 100644 --- a/tubearchivist/config/settings.py +++ b/tubearchivist/config/settings.py @@ -117,16 +117,16 @@ # names that people will guess, hence using first/last as well. global AUTH_LDAP_USER_ATTR_MAP_USERNAME - AUTH_LDAP_USER_ATTR_MAP_USERNAME = environ.get("TA_LDAP_USER_ATTR_MAP_USERNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_UID") or "uid", + AUTH_LDAP_USER_ATTR_MAP_USERNAME = environ.get("TA_LDAP_USER_ATTR_MAP_USERNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_UID") or "uid" global AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME - AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME = environ.get("TA_LDAP_USER_ATTR_MAP_PERSONALNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_FIRSTNAME") or "givenName", + AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME = environ.get("TA_LDAP_USER_ATTR_MAP_PERSONALNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_FIRSTNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_GIVENNAME") or "givenName" global AUTH_LDAP_USER_ATTR_MAP_SURNAME - AUTH_LDAP_USER_ATTR_MAP_SURNAME = environ.get("TA_LDAP_USER_ATTR_MAP_SURNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_LASTNAME") or "sn", + AUTH_LDAP_USER_ATTR_MAP_SURNAME = environ.get("TA_LDAP_USER_ATTR_MAP_SURNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_LASTNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_FAMILYNAME") or "sn" global AUTH_LDAP_USER_ATTR_MAP_EMAIL - AUTH_LDAP_USER_ATTR_MAP_EMAIL = environ.get("TA_LDAP_USER_ATTR_MAP_EMAIL") or environ.get("TA_LDAP_USER_ATTR_MAP_MAIL") or "mail", + AUTH_LDAP_USER_ATTR_MAP_EMAIL = environ.get("TA_LDAP_USER_ATTR_MAP_EMAIL") or environ.get("TA_LDAP_USER_ATTR_MAP_MAIL") or "mail" @@ -141,7 +141,7 @@ AUTH_LDAP_USER_SEARCH = LDAPSearch( AUTH_LDAP_USER_BASE, ldap.SCOPE_SUBTREE, - "(&(%(AUTH_LDAP_USER_ATTR_MAP_USERNAME)s=%(user)s)" + AUTH_LDAP_USER_FILTER + ")", + "(&("+AUTH_LDAP_USER_ATTR_MAP_USERNAME+"=%(user)s)" + AUTH_LDAP_USER_FILTER + ")", ) From 4dd82500180579939b9cc7c90331d12bba4952d4 Mon Sep 17 00:00:00 2001 From: "Brian C. Arnold" Date: Tue, 25 Oct 2022 11:54:50 -0400 Subject: [PATCH 6/9] Addressing lint error. --- tubearchivist/config/settings.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tubearchivist/config/settings.py b/tubearchivist/config/settings.py index 4298b9744..04b5714da 100644 --- a/tubearchivist/config/settings.py +++ b/tubearchivist/config/settings.py @@ -141,7 +141,11 @@ AUTH_LDAP_USER_SEARCH = LDAPSearch( AUTH_LDAP_USER_BASE, ldap.SCOPE_SUBTREE, - "(&("+AUTH_LDAP_USER_ATTR_MAP_USERNAME+"=%(user)s)" + AUTH_LDAP_USER_FILTER + ")", + "(&(" + + AUTH_LDAP_USER_ATTR_MAP_USERNAME + + "=%(user)s)" + + AUTH_LDAP_USER_FILTER + + ")", ) From 0d84899ea00655eff6c1763bf7a56ec92196c64a Mon Sep 17 00:00:00 2001 From: "Brian C. Arnold" Date: Tue, 25 Oct 2022 12:00:49 -0400 Subject: [PATCH 7/9] Finished updating formatting according to black linter. --- tubearchivist/config/settings.py | 55 ++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/tubearchivist/config/settings.py b/tubearchivist/config/settings.py index 04b5714da..f7e12b624 100644 --- a/tubearchivist/config/settings.py +++ b/tubearchivist/config/settings.py @@ -103,32 +103,47 @@ global AUTH_LDAP_BIND_PASSWORD AUTH_LDAP_BIND_PASSWORD = environ.get("TA_LDAP_BIND_PASSWORD") - # Attribute mapping options - # Since these are new environment variables, taking the opporunity to use more accurate - # env names. + # Since these are new environment variables, taking the opporunity to use more accurate + # env names. # Given Names are *_technically_* different from Personal names, as people who change their names # have different given names and personal names, and they go by personal names. - # Additionally, "LastName" is actually incorrect for many cultures, such as Korea, where the - # family name comes first, and the personal name comes last. - - # But we all know people are going to try to guess at these, so still want to include + # Additionally, "LastName" is actually incorrect for many cultures, such as Korea, where the + # family name comes first, and the personal name comes last. + + # But we all know people are going to try to guess at these, so still want to include # names that people will guess, hence using first/last as well. global AUTH_LDAP_USER_ATTR_MAP_USERNAME - AUTH_LDAP_USER_ATTR_MAP_USERNAME = environ.get("TA_LDAP_USER_ATTR_MAP_USERNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_UID") or "uid" + AUTH_LDAP_USER_ATTR_MAP_USERNAME = ( + environ.get("TA_LDAP_USER_ATTR_MAP_USERNAME") + or environ.get("TA_LDAP_USER_ATTR_MAP_UID") + or "uid" + ) global AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME - AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME = environ.get("TA_LDAP_USER_ATTR_MAP_PERSONALNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_FIRSTNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_GIVENNAME") or "givenName" + AUTH_LDAP_USER_ATTR_MAP_PERSONALNAME = ( + environ.get("TA_LDAP_USER_ATTR_MAP_PERSONALNAME") + or environ.get("TA_LDAP_USER_ATTR_MAP_FIRSTNAME") + or environ.get("TA_LDAP_USER_ATTR_MAP_GIVENNAME") + or "givenName" + ) - global AUTH_LDAP_USER_ATTR_MAP_SURNAME - AUTH_LDAP_USER_ATTR_MAP_SURNAME = environ.get("TA_LDAP_USER_ATTR_MAP_SURNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_LASTNAME") or environ.get("TA_LDAP_USER_ATTR_MAP_FAMILYNAME") or "sn" + global AUTH_LDAP_USER_ATTR_MAP_SURNAME + AUTH_LDAP_USER_ATTR_MAP_SURNAME = ( + environ.get("TA_LDAP_USER_ATTR_MAP_SURNAME") + or environ.get("TA_LDAP_USER_ATTR_MAP_LASTNAME") + or environ.get("TA_LDAP_USER_ATTR_MAP_FAMILYNAME") + or "sn" + ) global AUTH_LDAP_USER_ATTR_MAP_EMAIL - AUTH_LDAP_USER_ATTR_MAP_EMAIL = environ.get("TA_LDAP_USER_ATTR_MAP_EMAIL") or environ.get("TA_LDAP_USER_ATTR_MAP_MAIL") or "mail" - - + AUTH_LDAP_USER_ATTR_MAP_EMAIL = ( + environ.get("TA_LDAP_USER_ATTR_MAP_EMAIL") + or environ.get("TA_LDAP_USER_ATTR_MAP_MAIL") + or "mail" + ) global AUTH_LDAP_USER_BASE AUTH_LDAP_USER_BASE = environ.get("TA_LDAP_USER_BASE") @@ -141,15 +156,13 @@ AUTH_LDAP_USER_SEARCH = LDAPSearch( AUTH_LDAP_USER_BASE, ldap.SCOPE_SUBTREE, - "(&(" + - AUTH_LDAP_USER_ATTR_MAP_USERNAME + - "=%(user)s)" + - AUTH_LDAP_USER_FILTER + - ")", + "(&(" + + AUTH_LDAP_USER_ATTR_MAP_USERNAME + + "=%(user)s)" + + AUTH_LDAP_USER_FILTER + + ")", ) - - global AUTH_LDAP_USER_ATTR_MAP AUTH_LDAP_USER_ATTR_MAP = { "username": AUTH_LDAP_USER_ATTR_MAP_USERNAME, From a198f213774e260815725411999f3833d68e0425 Mon Sep 17 00:00:00 2001 From: "Brian C. Arnold" Date: Wed, 26 Oct 2022 12:34:58 -0400 Subject: [PATCH 8/9] Shortened comments to fit within line length. --- tubearchivist/config/settings.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/tubearchivist/config/settings.py b/tubearchivist/config/settings.py index f7e12b624..68818c44c 100644 --- a/tubearchivist/config/settings.py +++ b/tubearchivist/config/settings.py @@ -103,18 +103,22 @@ global AUTH_LDAP_BIND_PASSWORD AUTH_LDAP_BIND_PASSWORD = environ.get("TA_LDAP_BIND_PASSWORD") - # Attribute mapping options - - # Since these are new environment variables, taking the opporunity to use more accurate - # env names. - # Given Names are *_technically_* different from Personal names, as people who change their names - # have different given names and personal names, and they go by personal names. - # Additionally, "LastName" is actually incorrect for many cultures, such as Korea, where the - # family name comes first, and the personal name comes last. - - # But we all know people are going to try to guess at these, so still want to include - # names that people will guess, hence using first/last as well. + + """ + Since these are new environment variables, taking the opporunity to use + more accurate env names. + Given Names are *_technically_* different from Personal names, as people + who change their names have different given names and personal names, + and they go by personal names. Additionally, "LastName" is actually + incorrect for many cultures, such as Korea, where the + family name comes first, and the personal name comes last. + + But we all know people are going to try to guess at these, so still want + to include names that people will guess, hence using first/last as well. + """ + # Attribute mapping options + global AUTH_LDAP_USER_ATTR_MAP_USERNAME AUTH_LDAP_USER_ATTR_MAP_USERNAME = ( environ.get("TA_LDAP_USER_ATTR_MAP_USERNAME") From 09c06336c0f0a6ee7db32456caa670face6f8b9d Mon Sep 17 00:00:00 2001 From: simon Date: Thu, 27 Oct 2022 13:17:42 +0700 Subject: [PATCH 9/9] fix whitespace linting --- tubearchivist/config/settings.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tubearchivist/config/settings.py b/tubearchivist/config/settings.py index 68818c44c..9e5f016c2 100644 --- a/tubearchivist/config/settings.py +++ b/tubearchivist/config/settings.py @@ -103,12 +103,11 @@ global AUTH_LDAP_BIND_PASSWORD AUTH_LDAP_BIND_PASSWORD = environ.get("TA_LDAP_BIND_PASSWORD") - """ - Since these are new environment variables, taking the opporunity to use + Since these are new environment variables, taking the opporunity to use more accurate env names. Given Names are *_technically_* different from Personal names, as people - who change their names have different given names and personal names, + who change their names have different given names and personal names, and they go by personal names. Additionally, "LastName" is actually incorrect for many cultures, such as Korea, where the family name comes first, and the personal name comes last. @@ -118,7 +117,7 @@ """ # Attribute mapping options - + global AUTH_LDAP_USER_ATTR_MAP_USERNAME AUTH_LDAP_USER_ATTR_MAP_USERNAME = ( environ.get("TA_LDAP_USER_ATTR_MAP_USERNAME")