Allow ignoring the ppolicy extension

[viraptor]

Introduce ldap_use_ppolicy and allow disabling it to interact with providers that send broken ppolicy responses.
This fixes interaction with the Okta LDAP gateway.

[alexey-tikhonov]

FAIL: ipa_ldap_opt-tests
========================

Running suite(s): ipa_ldap_opt
[sssd] [sdap_extend_map] (0x0010): Attribute name (foo in LDAP) is already used by SSSD, please choose a different cache name
[sssd] [sdap_extend_map] (0x0010): Attribute uniqueID (bar in LDAP) is already used by SSSD, please choose a different cache name
84%: Checks: 13, Failures: 2, Errors: 0
src/tests/ipa_ldap_opt-tests.c:97:F:ipa_ldap_opt:test_compare_opts:0: [Numerical result out of range]
src/tests/ipa_ldap_opt-tests.c:179:F:ipa_ldap_opt:test_dp_opt_sentinel:0: Unexpected non-NULL for def_val.string in dp_option
FAIL ipa_ldap_opt-tests (exit status: 1)

FAIL: ad_ldap_opt-tests
=======================

Running suite(s): ad_ldap_opt
50%: Checks: 2, Failures: 1, Errors: 0
src/tests/ad_ldap_opt-tests.c:44:F:ad_ldap_opt:test_compare_opts:0: [Numerical result out of range]
FAIL ad_ldap_opt-tests (exit status: 1)

[viraptor]

How do I run those tests @alexey-tikhonov ? With make check, I get some unrelated failures, but also no ipa_ldap_opt-tests in the test-suite.log

[viraptor]

The covscan step fails with: "Error: Process completed with exit code 247." - there's no logs uploaded or details, so I'm not sure if that's an internal CI failure, or something I should address.

[viraptor]

I'm going to disagree with codefactor here :) That one function is pretty simple. Let me know if you really want to redesign it for this change.

[pbrezina]

Code looks good but there are more places where LDAP_CONTROL_PASSWORDPOLICYREQUEST is used. Also, can you squash the commits together, please?

We will also need to add some metadata, namely:

• Resolves: ticketurl
• release note, see https://github.com/SSSD/sssd/blob/master/.git-commit-template#L16

[viraptor]

I looked at the other sites using LDAP_CONTROL_PASSWORDPOLICYREQUEST. Password change needs fixing, another is in the response so can be ignored (since ppolicy was not requested in the first place), but the final in rebind I'm not sure what to do about.

It looks like the only way to properly implement it would be to push the use_ppolicy variable into the connection state. Or would it be enough to say that referrals=1 and use_ppolicy=0 are incompatible and fail at startup? Am I missing some other approach?

[pbrezina]

Hi, I just found some indentation requests.

Can you also modify this test and add parametrization to test use_ppolicy = false|true?

[pbrezina]

I looked at the other sites using LDAP_CONTROL_PASSWORDPOLICYREQUEST. Password change needs fixing, another is in the response so can be ignored (since ppolicy was not requested in the first place), but the final in rebind I'm not sure what to do about.

It looks like the only way to properly implement it would be to push the use_ppolicy variable into the connection state. Or would it be enough to say that referrals=1 and use_ppolicy=0 are incompatible and fail at startup? Am I missing some other approach?

Adding it to state sounds like a good approach. Correct me if I'm wrong, but you already did this in the latest version of your patch, right?

[viraptor]

you already did this in the latest version of your patch, right?

Yup, turned out much less of an issue than I expected (doesn't need to be passed between request/response)

[thalman]

The patch looks good. Please fix the indentation as Pavel already pointed out, so we can merge it.

It would be also nice if you also rebase to the current mater so we can see latest tests on this PR - but this is not strictly needed.

[thalman]

LGTM, thanks for the effort

[alexey-tikhonov]

_____________________ ERROR collecting tests/test_ldap.py ______________________
In test_ldap__change_password: function uses no argument 'ldap_use_ppolicy'

[pbrezina]

_____________________ ERROR collecting tests/test_ldap.py ______________________
In test_ldap__change_password: function uses no argument 'ldap_use_ppolicy'

I took the liberty and fixed this, I also added this argument to other relevant test cases. Let's wait for PR CI.

[alexey-tikhonov]

Pushed PR: #6904

• master
  • 1980e2c - LDAP: Allow ignoring the ppolicy extension