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

Update get_attr_filter in LDAPSearch to handle nsaccountlock user searches #688

Closed

Conversation

redhatrises
Copy link
Contributor

@redhatrises redhatrises commented Apr 3, 2017

  • Update get_attr_filter in LDAPSearch to handle nsaccountlock by setting the default value for
    nsaccountlock to false as well as update the filter to check for the default value

  • https://pagure.io/freeipa/issue/6896

if name == 'nsaccountlock' and value == False:
# If nsaccountlock is False, set the value to True and
# search for nsaccountlock != True. This way we can search
# for any users where nsaccountlock=False or nsaccountlock=None.
Copy link
Contributor

Choose a reason for hiding this comment

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

No magic special cases please, the whole point of doing this in baseldap is that the fix would be generic. If the bit below doesn't work, we need to figure out how to make it work.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, figured it out:

  • The nsaccountlock param in user plugin does not have a default value specified.
  • The check for default value below should actually look like this:
                default = self.get_default_of(name, **options)
                if default is not None and value == default:
                    fltr = ldap.combine_filters([fltr, '(!({}=*))'.format(name)])

@redhatrises
Copy link
Contributor Author

@HonzaCholasta ready for your review.

@redhatrises
Copy link
Contributor Author

Bump for review

@redhatrises
Copy link
Contributor Author

Should this also go into the 4.5 branch?

@HonzaCholasta
Copy link
Contributor

I guess it should. Could you please file a ticket?

@redhatrises
Copy link
Contributor Author

@HonzaCholasta
Copy link
Contributor

Thanks!

@HonzaCholasta HonzaCholasta added ack Pull Request approved, can be merged and removed ack Pull Request approved, can be merged labels Apr 25, 2017
@HonzaCholasta
Copy link
Contributor

Actually, please remove the change in VERSION.m4, as it is not necessary and prevents the patch from applying cleanly on top of ipa-4-5.

Also please add the ticket link to commit messages.

…rches

- Update get_attr_filter in LDAPSearch to handle nsaccountlock by setting the default value for
  nsaccountlock to false as well as update the filter to check for the default value
- Remove pytest xfail for test_find_enabled_user

https://pagure.io/freeipa/issue/6896
@redhatrises
Copy link
Contributor Author

@HonzaCholasta I removed the change to VERSION.m4

@HonzaCholasta HonzaCholasta added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels Apr 26, 2017
@HonzaCholasta
Copy link
Contributor

master:

  • 38276d3 Update get_attr_filter in LDAPSearch to handle nsaccountlock user searches

ipa-4-5:

  • dc4d60c Update get_attr_filter in LDAPSearch to handle nsaccountlock user searches

@redhatrises redhatrises deleted the ldap_search_nsaccountlock branch April 26, 2017 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
2 participants