-
Notifications
You must be signed in to change notification settings - Fork 120
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
Separate LDAP lookup fields from User lookup fields #273
Comments
Thank you for taking the time to describe your issue.
I can see the utility of using different fields to bind vs lookup.
The conflict resolution logic seems to be quite specific to your use-case
though, and requires write access to the LDAP server. I think many users
would find that surprising.
The problem here is that the username is being used as a source of
uniqueness in two different databases. Changing usernames is generally
problematic in this scenario, and the resolution is, I suspect, going to be
hard to generalise usefully.
…On Fri, 3 May 2024 at 18:55, jholladay10 ***@***.***> wrote:
It seems that the LDAP lookup names are the same as the User lookup names
(both taken from LDAP_AUTH_USER_LOOKUP_FIELDS. I'd like them to be allowed
to be separate.
My scenario occurs with Active Directory. I don't know whether it can
occur with OpenLDAP.
As an example, I would like to use sAMAccount name to authenticate as AD,
but use the object GUID to look up the user. That way, if a user's
sAMAccountName changes, their permissions will remain associated with their
object guid instead of losing their permissions or, worse, assuming someone
else's permissions who previously had that sAMAccount name. I think the
same could apply to upn. This does introduce the edge case of an "old"
username conflicting with a "new" username in the database.
To address this, i would:
1. Create a setting LDAP_AUTH_USER_BIND_FIELDS
2. Use this value or, if empty, use LDAP_AUTH_USER_LOOKUP_FIELDS, when
binding to LDAP
3. When getting the user, if the user lookup fields are different than
the bind fields, look up the user on each set and compare them.
4. If the user object is the same or the username lookup failed to
retrieve an object, then there is no conflict and the current behavior is
fine.
5. If there is a conflict, then the current object guid can retain the
current LDAP lookup fields, and the LDAP lookup fields for the other user
object need to be updated to eliminate the conflict. A default AD
implementation would be to look up the conflicting user on their object
guid and update their user record to reflect the current sAMAccountName for
that object.
—
Reply to this email directly, view it on GitHub
<#273>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABEKCG6ZZHPHONJCFEAU4DZAPFSFAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI3TQMJWGM2TCOI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Hi, Dave.
I've forked the repository and made some suggested changes. I'm planning to
submit a pull request for your review and discussion. I"m sure we can
arrive at something cool.
Thanks!
Jason
…On Thu, May 9, 2024 at 11:49 AM Dave Hall ***@***.***> wrote:
Thank you for taking the time to describe your issue.
I can see the utility of using different fields to bind vs lookup.
The conflict resolution logic seems to be quite specific to your use-case
though, and requires write access to the LDAP server. I think many users
would find that surprising.
The problem here is that the username is being used as a source of
uniqueness in two different databases. Changing usernames is generally
problematic in this scenario, and the resolution is, I suspect, going to
be
hard to generalise usefully.
On Fri, 3 May 2024 at 18:55, jholladay10 ***@***.***> wrote:
> It seems that the LDAP lookup names are the same as the User lookup
names
> (both taken from LDAP_AUTH_USER_LOOKUP_FIELDS. I'd like them to be
allowed
> to be separate.
>
> My scenario occurs with Active Directory. I don't know whether it can
> occur with OpenLDAP.
>
> As an example, I would like to use sAMAccount name to authenticate as
AD,
> but use the object GUID to look up the user. That way, if a user's
> sAMAccountName changes, their permissions will remain associated with
their
> object guid instead of losing their permissions or, worse, assuming
someone
> else's permissions who previously had that sAMAccount name. I think the
> same could apply to upn. This does introduce the edge case of an "old"
> username conflicting with a "new" username in the database.
>
> To address this, i would:
>
> 1. Create a setting LDAP_AUTH_USER_BIND_FIELDS
> 2. Use this value or, if empty, use LDAP_AUTH_USER_LOOKUP_FIELDS, when
> binding to LDAP
> 3. When getting the user, if the user lookup fields are different than
> the bind fields, look up the user on each set and compare them.
> 4. If the user object is the same or the username lookup failed to
> retrieve an object, then there is no conflict and the current behavior
is
> fine.
> 5. If there is a conflict, then the current object guid can retain the
> current LDAP lookup fields, and the LDAP lookup fields for the other
user
> object need to be updated to eliminate the conflict. A default AD
> implementation would be to look up the conflicting user on their object
> guid and update their user record to reflect the current sAMAccountName
for
> that object.
>
> —
> Reply to this email directly, view it on GitHub
> <#273>, or
> unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AABEKCG6ZZHPHONJCFEAU4DZAPFSFAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI3TQMJWGM2TCOI>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
—
Reply to this email directly, view it on GitHub
<#273 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AL6RLJJBFCKI42EKOTWLHS3ZBOLHTAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBSHEZTEOJZGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Nice! I'm away for a week right now, and have a new baby to look after when
I'm back. But I will take a look when I can!
Consider implementing you conflict resolution logic as an extensible noop
hook, since that's the part that feels least general here.
…On Fri, 10 May 2024 at 01:15, jholladay10 ***@***.***> wrote:
Hi, Dave.
I've forked the repository and made some suggested changes. I'm planning
to
submit a pull request for your review and discussion. I"m sure we can
arrive at something cool.
Thanks!
Jason
On Thu, May 9, 2024 at 11:49 AM Dave Hall ***@***.***> wrote:
> Thank you for taking the time to describe your issue.
>
> I can see the utility of using different fields to bind vs lookup.
>
> The conflict resolution logic seems to be quite specific to your
use-case
> though, and requires write access to the LDAP server. I think many users
> would find that surprising.
>
> The problem here is that the username is being used as a source of
> uniqueness in two different databases. Changing usernames is generally
> problematic in this scenario, and the resolution is, I suspect, going to
> be
> hard to generalise usefully.
>
> On Fri, 3 May 2024 at 18:55, jholladay10 ***@***.***> wrote:
>
> > It seems that the LDAP lookup names are the same as the User lookup
> names
> > (both taken from LDAP_AUTH_USER_LOOKUP_FIELDS. I'd like them to be
> allowed
> > to be separate.
> >
> > My scenario occurs with Active Directory. I don't know whether it can
> > occur with OpenLDAP.
> >
> > As an example, I would like to use sAMAccount name to authenticate as
> AD,
> > but use the object GUID to look up the user. That way, if a user's
> > sAMAccountName changes, their permissions will remain associated with
> their
> > object guid instead of losing their permissions or, worse, assuming
> someone
> > else's permissions who previously had that sAMAccount name. I think
the
> > same could apply to upn. This does introduce the edge case of an "old"
> > username conflicting with a "new" username in the database.
> >
> > To address this, i would:
> >
> > 1. Create a setting LDAP_AUTH_USER_BIND_FIELDS
> > 2. Use this value or, if empty, use LDAP_AUTH_USER_LOOKUP_FIELDS, when
> > binding to LDAP
> > 3. When getting the user, if the user lookup fields are different than
> > the bind fields, look up the user on each set and compare them.
> > 4. If the user object is the same or the username lookup failed to
> > retrieve an object, then there is no conflict and the current behavior
> is
> > fine.
> > 5. If there is a conflict, then the current object guid can retain the
> > current LDAP lookup fields, and the LDAP lookup fields for the other
> user
> > object need to be updated to eliminate the conflict. A default AD
> > implementation would be to look up the conflicting user on their
object
> > guid and update their user record to reflect the current
sAMAccountName
> for
> > that object.
> >
> > —
> > Reply to this email directly, view it on GitHub
> > <#273>, or
> > unsubscribe
> > <
>
https://github.com/notifications/unsubscribe-auth/AABEKCG6ZZHPHONJCFEAU4DZAPFSFAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI3TQMJWGM2TCOI>
>
> > .
> > You are receiving this because you are subscribed to this
thread.Message
> > ID: ***@***.***>
> >
>
> —
> Reply to this email directly, view it on GitHub
> <
#273 (comment)>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AL6RLJJBFCKI42EKOTWLHS3ZBOLHTAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBSHEZTEOJZGM>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
—
Reply to this email directly, view it on GitHub
<#273 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABEKCCDRVNNMFGHPZRNBBLZBQGQHAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBTGYZTGOBZGM>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Congratulations!
I'm off this week, so i can only code when my wife is sleeping or
distracted :o) So I probably won't be submitting the pull request until
next week anyway.
<<Consider implementing you conflict resolution logic as an extensible noop
hook, since that's the part that feels least general here.>>
Absolutely! It's optional behavior that only has an effect if you specify
a "bind fields" setting and only if the bind fields are different than the
lookup fields. I wrote a resolution function that works in my case and
included it as a specific active directory function like the others that
are in there.
I also added functionality for a field-level cleaner setting (so cleaner
functions can be composed via setting, maybe overkill?) and wrote a cleaner
for the AD uuid that makes the value usable by the django orm layer. As a
newbie to the code, I renamed some parameters from kwargs to model_fields
to make it easier to understand what was going on.
The thing i've done that most noticeably departs from the existing code
style is that i created a class in utils to encapsulate the behaviors
around lookup fields vs bind fields vs update fields, and also mapping of
fields between ldap and model. I refactored the code that was doing things
like this to leverage that class. I hope you like it when you see it. If
not, I'll understand.
…On Fri, May 10, 2024 at 2:47 AM Dave Hall ***@***.***> wrote:
Nice! I'm away for a week right now, and have a new baby to look after
when
I'm back. But I will take a look when I can!
Consider implementing you conflict resolution logic as an extensible noop
hook, since that's the part that feels least general here.
On Fri, 10 May 2024 at 01:15, jholladay10 ***@***.***> wrote:
> Hi, Dave.
>
> I've forked the repository and made some suggested changes. I'm planning
> to
> submit a pull request for your review and discussion. I"m sure we can
> arrive at something cool.
>
> Thanks!
> Jason
>
> On Thu, May 9, 2024 at 11:49 AM Dave Hall ***@***.***> wrote:
>
> > Thank you for taking the time to describe your issue.
> >
> > I can see the utility of using different fields to bind vs lookup.
> >
> > The conflict resolution logic seems to be quite specific to your
> use-case
> > though, and requires write access to the LDAP server. I think many
users
> > would find that surprising.
> >
> > The problem here is that the username is being used as a source of
> > uniqueness in two different databases. Changing usernames is generally
> > problematic in this scenario, and the resolution is, I suspect, going
to
> > be
> > hard to generalise usefully.
> >
> > On Fri, 3 May 2024 at 18:55, jholladay10 ***@***.***> wrote:
> >
> > > It seems that the LDAP lookup names are the same as the User lookup
> > names
> > > (both taken from LDAP_AUTH_USER_LOOKUP_FIELDS. I'd like them to be
> > allowed
> > > to be separate.
> > >
> > > My scenario occurs with Active Directory. I don't know whether it
can
> > > occur with OpenLDAP.
> > >
> > > As an example, I would like to use sAMAccount name to authenticate
as
> > AD,
> > > but use the object GUID to look up the user. That way, if a user's
> > > sAMAccountName changes, their permissions will remain associated
with
> > their
> > > object guid instead of losing their permissions or, worse, assuming
> > someone
> > > else's permissions who previously had that sAMAccount name. I think
> the
> > > same could apply to upn. This does introduce the edge case of an
"old"
> > > username conflicting with a "new" username in the database.
> > >
> > > To address this, i would:
> > >
> > > 1. Create a setting LDAP_AUTH_USER_BIND_FIELDS
> > > 2. Use this value or, if empty, use LDAP_AUTH_USER_LOOKUP_FIELDS,
when
> > > binding to LDAP
> > > 3. When getting the user, if the user lookup fields are different
than
> > > the bind fields, look up the user on each set and compare them.
> > > 4. If the user object is the same or the username lookup failed to
> > > retrieve an object, then there is no conflict and the current
behavior
> > is
> > > fine.
> > > 5. If there is a conflict, then the current object guid can retain
the
> > > current LDAP lookup fields, and the LDAP lookup fields for the other
> > user
> > > object need to be updated to eliminate the conflict. A default AD
> > > implementation would be to look up the conflicting user on their
> object
> > > guid and update their user record to reflect the current
> sAMAccountName
> > for
> > > that object.
> > >
> > > —
> > > Reply to this email directly, view it on GitHub
> > > <#273>, or
> > > unsubscribe
> > > <
> >
>
https://github.com/notifications/unsubscribe-auth/AABEKCG6ZZHPHONJCFEAU4DZAPFSFAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI3TQMJWGM2TCOI>
>
> >
> > > .
> > > You are receiving this because you are subscribed to this
> thread.Message
> > > ID: ***@***.***>
> > >
> >
> > —
> > Reply to this email directly, view it on GitHub
> > <
>
#273 (comment)>,
>
> > or unsubscribe
> > <
>
https://github.com/notifications/unsubscribe-auth/AL6RLJJBFCKI42EKOTWLHS3ZBOLHTAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBSHEZTEOJZGM>
>
> > .
> > You are receiving this because you authored the thread.Message ID:
> > ***@***.***>
> >
>
> —
> Reply to this email directly, view it on GitHub
> <
#273 (comment)>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AABEKCCDRVNNMFGHPZRNBBLZBQGQHAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBTGYZTGOBZGM>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
>
—
Reply to this email directly, view it on GitHub
<#273 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AL6RLJPGTDCAFXNPTS2GBHDZBRUOPAVCNFSM6AAAAABHF5GLWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBTHE3TMMBZGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It seems that the LDAP lookup names are the same as the User lookup names (both taken from LDAP_AUTH_USER_LOOKUP_FIELDS. I'd like them to be allowed to be separate.
My scenario occurs with Active Directory. I don't know whether it can occur with OpenLDAP.
As an example, I would like to use sAMAccount name to authenticate as AD, but use the object GUID to look up the user. That way, if a user's sAMAccountName changes, their permissions will remain associated with their object guid instead of losing their permissions or, worse, assuming someone else's permissions who previously had that sAMAccount name. I think the same could apply to upn. This does introduce the edge case of an "old" username conflicting with a "new" username in the database.
To address this, i would:
The text was updated successfully, but these errors were encountered: